osuakatsuki / bancho.py Goto Github PK

I was thinking of creating a shell script which will handle the entire installation process with proper error handling. What do you think?

This is actually a problem with every server implementation so far I believe; if multiple players achieve first place on a map, it'll announce all of them in the order that it gets the scores in.

The server should enqueue a task once the server knows the match has ended to re-check the first place in a few seconds (~5-10 should be fine), and announce it if it's been set.

very basic tournament client support is being rolled out in 3.2.4, but it's very undertested and certainly has some issues.
gulag was designed without the tourney client in mind (i've never really used it), so having a player online multiple times will likely cause some issues in places where i don't expect it.

we should also probably limit the amount of tourney clients that a user connects with at a time, and perhaps do some more advanced safety checks.

Proposal

Summary

The offical osu! server has an IRC server. So does Ripple. They provide a platform for external bots such as Tillerino and MaidBot to operate, enriching the user experience on their respective server. I propose we do the same, although make it async.

Motivation

A lot of a private server's job is to emulate Bancho while also adding new features. Even though IRC is not essential to a Bancho clone, it however allows bots to run without needing a Bancho connection. Mostly, this serves as compatibility, since most osu bot frameworks work via IRC (i.e. bancho.js). Also, it's cool.

Describe alternatives you've considered

Using a "modern" IRC-like service (like Firebase) would render the compatibility aspect useless. The only purpose IRC serves on osu! is interacting with osu's chat. It can't submit scores, or even lookup user data. Simple is good.

Additional context

I'm willing to develop it as @cmyui already has enough on his plate. Think of it as an initiation project. If it never gets delivered, it won't cripple Gulag's infrastructure. Likewise, if it does get delivered it (should) warrant a version increase (@cmyui can i commit to devel?)

Some objectives for the IRC server

• Seamless integration into existing code (Bancho user == IRC user)
• irc.ppy.sh compatible
• Redirects DMs to BanchoBot to Aiko
• Token protected
• DB table for storing IRC usernames and tokens (with a relational element to userids)
• gulag-web interface for creating tokens (talk with @yo-ru about that one)
• command for creating tokens
• async (runs in main loop instead of threads, because screw threads)
• lightweight and fast (ad hoc, not a thin layer over a dependency)
• works with IRC clients like HexChat

-- sansquick

At the moment I use a thin aiomysql pool wrapper with fetch()/execute() functions which open a connection & retrieve a cursor to perform a single query at a time before closing them.

This is okay most of the time since the conn/cursor retrieval portion of the queries are pretty cheap, but there are some times in the code where I do many back-to-back queries and the cost adds up unnecessarily.

I'll soon be pushing an update to cmyui_pkg to make this simpler & more elegant and the rewriting will begin after that.

while the server can already use chimu.moe for osu!direct search (and i plan on adding some more advanced search mechanics for map stats like ar/od/etc.), the api for chimu's downloading is a bit different from cheesegull and also requires an API key, so we'll have to implement that somewhere in config if we want to fully support it.

Direct search is work but when i doubleclick and download it it's gone and in console it's show this thing

If a map is declined (or even accepted), it should enqueue mail to the requestee telling them that either their request was accepted (with the new status of the map), or the reason it was declined.

BENEFITS : Better collaboration and software quality.

it seems whenever connecting to the server after a while of inactivity, the osu bansho connection takes a long time (like half to one minute) and there's this error printed to console

As per #111, I wrote a helper script for gulag: https://github.com/ospensory/gulag/commit/61b1f58dbbad6520c9a620940be1238dc6d9aa0d. Let me know about your thoughts. If it's merge-able then I need to rework on the script to handle the existing directory.

Should be pretty simple, perhaps the existing friendships table should be modified a bit to be dual-purpose for this. A user should not be able to be friends with a user they have blocked, and vice-versa.

• File: main.py
• Line(s):
  • main.py#L126
  • main.py#L127

Description

A lambda that calls a function without modifying any of its parameters is unnecessary. Python functions are first-class objects and can be passed around in the same way as the resulting lambda. It is recommended to remove the lambda and use the function directly. Example:

loop.add_signal_handler(signal.SIGINT, loop.stop)
loop.add_signal_handler(signal.SIGTERM, loop.stop)

Occurrences

There are 2 occurrences of this issue in the repository.

See all occurrences on DeepSource → deepsource.io/gh/Uniminin/gulag/issue/PYL-W0108/occurrences/

not really necessary and just a suggestions but maybe add command to download pip modules :3

BTW I LOVE THE PROJECT, GREAT WORK YOU'VE DONE !! WW

https://osu.ppy.sh/wiki/en/Performance_points#how-much-bonus-pp-is-awarded-for-having-lots-of-scores-on-ranked-maps?

• ubuntu 20.04 & nginx have unknown issues? i recommend using 18.04

apparently osu uses older tls 1.2 or 1.1 which ubuntu 20.04 doesn't support but 18.04 does.

I tried adding

ssl_protocols TLSv1 TLSv1.1 TLSv1.2 TLSv1.3;
ssl_ciphers "EECDH+AESGCM:EDH+AESGCM:AES256+EECDH:AES256+EDH:@SECLEVEL=1";

to nginx config but it didn't work. edit: not sure where I tried it (i think on centos 8) but it worked on 20.04

So if you can somehow enable TLS 1.0/1.1 then it should work.

Alternatively you could just use cloudflare, which supports it.

These should be replaced by a system that logs the stacktraces (and automatically sends them to cmyui if enabled), as freezing the entire server is no longer a viable option with gulag starting to be used by 'real' production servers. (it'll only scale worse with time)

Basically stuff like this should be done with a single connection & query to sql, then the loop should just be in python to move the data around in memory.

gulag is missing email verification functionality.

Can be fetched from args while score submission. Argument called "st".

root@ubuntu-s-2vcpu-4gb-sfo3-01:~/gulag# ./main.py
[08:03:49PM] It is not recommended to run gulag as root, especially in production..
gulag v3.5.4 ran into an issue before starting up :(
Traceback (most recent call last):
File "/root/gulag/./main.py", line 209, in
raise SystemExit(loop.run_until_complete(main()))
File "uvloop/loop.pyx", line 1501, in uvloop.loop.Loop.run_until_complete
File "/root/gulag/./main.py", line 126, in main
await misc.utils.update_mysql_structure()
File "/root/gulag/misc/utils.py", line 691, in update_mysql_structure
if latest_ver == current_ver:
File "/usr/local/lib/python3.9/dist-packages/cmyui/version.py", line 22, in eq
return self.as_tuple == other.as_tuple
AttributeError: 'NoneType' object has no attribute 'as_tuple'

Codebase is getting really quite large now and errors do arise and are left unnoticed for longer than ideal, it's time for gulag to get some tests.

Is gulag hardly dependent on python3.9? Does it work on python3.7?

I was wondering weather gulag would run with pypy!

At the moment, when a user submits a new pp-awarding score, only their rank will be updated, rather than all the users inbetween the old and new rank.

example: Hello World and Hello_World, they both get the safe name hello_world and if someone tries to make one of either after the other already exists it throws an unhandled exception.

this would be a better check for username, it shouldn't matter whether or not the normal name already exists.

    if 'username' not in errors:
        await db_cursor.execute('SELECT 1 FROM users WHERE safe_name = %s', [name.lower().replace(' ', '_')])
        if db_cursor.rowcount != 0:
            errors['username'].append('Username already taken by another player.')

Description

return statement causes the control flow to be disrupted, making the else / elif block here unnecessary. This doesn't mean you can not use it, but it is recommended to refactor this for a better readability. Bad:

def classify_number(x): if x % 2 == 0: return "Even" else: return …

Occurrences

There are 9 occurrences of this issue in the repository.

See all occurrences on DeepSource → deepsource.io/gh/Uniminin/gulag/issue/PYL-R1705/occurrences/

I recommend adding last seen timestamp to output of /api/get_player_status?id={id} when user is offline

Description

Comparison with a callable has been detected, which suggests you may have forgotten the parentheses to actually call the function or method. If you intended to check if both of these callables are the same, it's recommended to use the is operator for comparison.

Occurrences

There are 4 occurrences of this issue in the repository.

See all occurrences on DeepSource → deepsource.io/gh/Uniminin/gulag/issue/PYL-W0143/occurrences/

When I first started designing, I didn't think I'd be making such frequent changes to the structure of the database; turns out this happens much more than I was anticipating, and leaving it on users to make their own queries to fix their sql isn't great..

Will probably just make a log file with copy-pastable updates for each version release with sql changes

currently the implementation uses subprocess, which is pretty gross but works fine for the time being.

this poses some challenges, i've had issues whenever i've tried to use the bindings in an efficient manner (reusing the same instance many times) for mass-recalculations - recently i've been in contact with the main developer and we've been working our way through these slowly, though.

we'll also need to build our own bindings since we've modified oppai-ng's source.

I spoke to cmyui about this in DMs and it seems to be a problem with the web handler, but I thought it was a good idea to post it here for various reasons

Multiplayer chat is still listed in channels after leaving a multiplayer lobby

What's GraphQL

Taken straight from their website:

GraphQL is a query language for APIs and a runtime for fulfilling those queries with your existing data
-GraphQL

Basically, instead of individual endpoints representing objects in the server (ie GET getUser/1 == Player(1)) , GraphQL processes a scheme sent my the client and fills it out with the requested data. This allows our entire api to be one endpoint. You can read more about it on their website.

Here is an example of a GraphQL request and response.

GraphQL is mainly aimed towards sending data from objects. You can request attributes of an object and it will respond with those attributes.

Why

As gulag-web requires more and more data from data, there is a need to keep creating endpoints for every unique information requested. This creates a lot of bloat and unnecessary version communication between gulag and gulag-web. A GraphQL implementation would cut down on this bloat and make development much easier. Most API transactions are about lbs, users, or other data of that nature, which can be simplified with this API system.

On top of this, the server only responses with the data requested. Systems like this would become archaic.
https://github.com/cmyui/gulag/blob/b0a87c123223c5498428169c4a43a9ecd731f16f/domains/osu.py#L1563-L1584

How

Graphene is a robust GraphQL engine for python. Instead of running as a server, it processes string information you pass into it. This allows Graphene to run over cmyui_pkg. With a bit of modification, existing objects can be used by Graphene to make GraphQl schema. On gulag-web, Graphene can be used to parse responses into a dict which can be used by the program. Either way win-win.

Thoughts

While I know GraphQL for its powerful requesting functionality, I also think it can "POST" data too. Maybe we can dip our toes into GraphQL on the "GET" side before using it for posting data, idk.

EDIT: super cool demo https://api.spacex.land/graphql/

gulag-web has one how come we don't. hmph

Since its not legally murky, it will prevent plenty of headaches.

edit: mainly for localhost applications since the same objective can be achieved with Cloudflare and/or LetsEncrypt.

The code is beautiful and well written!
It's the first time I felt reading other people's code is so comfortable

The API is currently very lacking; we need at least coverage of user information (stats, scores, achievements, info), beatmaps, clans, etc.), we plan to move the API back to gulag itself mostly due to it's caching.

I need to put more thought into the optimal design for things like ranking graph data.

Description

At line handlers.py#L229, One of known pre Python 2.5 ternary syntax is being used. It is recommended to use an if expression like: X if condition else Y. Using [condition] and [on_true] or [on_false] may also give wrong results when on_true has a false boolean value. The inconsistency: Consider the following:

path = _path if os.path.exists(_path) else default_avatar

Occurrences

There is just only 1 occurrence of this issue in the repository.

See all occurrences on DeepSource → deepsource.io/gh/Uniminin/gulag/issue/PYL-R1706/occurrences/

File: https://raw.githubusercontent.com/cmyui/gulag/master/requirements.txt

It's better to specify what python module versions to pull instead of pulling the latest version.
Things may not work depending on the module versions used on/for development/production and the latest one used by user(s).

To avoid such issues it is strongly recommended to specify all module versions.

Note: pip does not have true dependency resolution.

• Affected Line: commands.py#L365

Description

If the except block catches a very general exception, it is likely to include many unrelated errors too. Try to be more precise while catching the exception.

Occurrences

There is 1 occurrence of this issue in the repository.

See all occurrences on DeepSource → deepsource.io/gh/Uniminin/gulag/issue/PYL-W0703/occurrences/

!compare is a command that's become commonplace in osu!-related discord bots.
lets you compare to the most recent !top or !recent post in the channel.

• File: commands.py
• Line(s):
  • commands.py#L46
  • commands.py#L152

Description

Defining a local variable or function with the same name as a built-in object makes the built-in object unusable within the current scope and makes the code prone to bugs.

Occurrences

There are 2 occurrences of this issue in the repository.

See all occurrences on DeepSource → deepsource.io/gh/Uniminin/gulag/issue/PYL-W0622/occurrences/

This one is pretty large compared to any other issues, it entails the implementation of the following handlers:

POST /web/osu-osz2-bmsubmit-post.php
POST /web/osu-osz2-bmsubmit-upload.php
GET /web/osu-osz2-bmsubmit-getid.php
GET /web/osu-get-beatmap-topic.php

along with writing a program to write osz2 files, and quite a few backend changes.
I've already done a few hundred lines of code of this one, but it will likely not be implemented into master for a while to come.

that's useful to recalc pp in all maps to live test changes made in oppai-ng <3

Reasons

• Better Archiving
• More """professional"""
• Better changelogs

At the moment, all documentation of the API is what is provided in the code.

This is mostly fine since we're in python and the implementation is quite simple at the moment, but eventually the docs should be moved into a section of their own, unrelated to the code to help our community developers out.

While first designing, I thought I could simply ignore the map name and fetch it from the db (or it's cache wrapper) by md5 (ends up being faster this way), but this also means people can't play unsubmitted maps in multi.. while this doesn't seem major at all, I like playing speedup diffs in multi matches so this is simply unacceptable :D

Should be a pretty ez fix, just needs a recode.

This problem causes some beatmap file names with special characters :\*></?" | were incorrectly determined for ranking status.

The problem occurred in: osu-osz2-getscores.php [domains/osu.py - getScores]
https://github.com/cmyui/gulag/blob/4de101c800516b1698de13db2a15df1d2933ff48/domains/osu.py#L913

Description

These are the code snippets where the problem occurred:
https://github.com/cmyui/gulag/blob/4de101c800516b1698de13db2a15df1d2933ff48/domains/osu.py#L967-L992
Notice that line 967, gulag uses a regex parsing the beatmap filename sent by the osu client:
https://github.com/cmyui/gulag/blob/4de101c800516b1698de13db2a15df1d2933ff48/domains/osu.py#L967
If parsed correctly, artist, title, creator, version will be extracted from the beatmap filename we received. (There is a bug in the regex, the bug prevents correct parsing, I have submitted the pr here: #78 )

Assuming here that the bug has been resolved, we continue...

The main problem arises in the following code snippet:
https://github.com/cmyui/gulag/blob/4de101c800516b1698de13db2a15df1d2933ff48/domains/osu.py#L975-L982
gulag tries to get the beatmap using the information(artist, title, version, creator) obtained by parsing filename,
but, filename will not include the following characters: :\*></?" |.

Beatmap info (from the osu!api or database) will contain these special characters, this causes the beatmap with the special characters to not match correctly, such as this: https://osu.ppy.sh/beatmapsets/1084205#osu/2267413

diff name: [[[#340000_cyiorlia//alt.remix]]]_Un:Re:sponsive oh..its... but still ranked.

Solutions

Join the beatmap information from the database or osu!api into legal filenames (also removing special characters) so that the filenames can be successfully matched.