search

LeaPhant / flowabot

Modular Discord bot with fun features including twitch commands and advanced osu! commands. 🌷
MIT License
111 stars 33 forks source link

Using !render on some maps causes the entire bot to halt. #19

Closed Kodehawa closed 5 years ago

Kodehawa commented 5 years ago

For example, using !render 20s on https://osu.ppy.sh/b/1529760 makes the entire bot halt for a few minutes.

Maybe other maps could cause the same issue.

Possible fixes could be either excluding Aspire maps or properly implement multithreading on command handling.

Also some of the slider positions of those maps are basically going from inf pos to inf neg. Might want to add an edge case check?

Resuming: Any command that takes too much to answer causes the bot to halt for the duration of the command until it replies. Please implement command multi-threading if possible, or in this case Promise-based commands could work too.

Maaster commented 5 years ago

Just to chime in on this one, I can't reproduce this:

[2019-07-14T19:53:02.387Z] Maaster : !score https://osu.ppy.sh/beatmapsets/537048#osu/1529760 [2019-07-14T19:53:02.391Z] returning data for username /tmp/osumaps/1529760.osu [2019-07-14T19:53:03.868Z] getting ur [2019-07-14T19:53:04.539Z] flowabot : [2019-07-14T19:53:08.908Z] Maaster : !render 20s [2019-07-14T19:53:08.910Z] render length 20 [2019-07-14T19:53:08.910Z] specified ar undefined [2019-07-14T19:53:11.885Z] score has replay process beatmap: 3497.642ms [2019-07-14T19:53:12.409Z] downloading beatmap osz [2019-07-14T19:53:13.486Z] flowabot : Rendering... [2019-07-14T19:53:13.543Z] downloading from https://osu.gatari.pw/d/537048 [2019-07-14T19:53:15.326Z] resizing image render beatmap: 10714.129ms encode video: 8565.589ms [2019-07-14T19:53:36.090Z] flowabot :

Takes around 30s for me (which is only slightly longer than other maps, in general I need around 15-20s), not multiple minutes.

I agree tho, if it is a problem for others.

Kodehawa commented 5 years ago

tbh, the guy hosting it is doing so in an old laptop. Still quite an issue if it leaves the bot unusable tho

LeaPhant commented 5 years ago

I've just seen this issue and I've implemented circular sliders 2 hours ago in https://github.com/LeaPhant/flowabot/commit/4e15e1ee183d1a3dc749707b68b4c27355933090. Seems like this map works now.

LeaPhant commented 5 years ago

I totally missed the multi-threading part last night. it's already implemented for rendering and encoding but not yet for processing beatmaps which can take long on maps like this I guess.

I'll just appropriate this issue to track the progress of implementing that. :)

LeaPhant commented 5 years ago

Alright, beatmap processing is fully multithreaded as of https://github.com/LeaPhant/flowabot/commit/a16f1ae61f7e1d07092d3c13ba3cb1fa044c8efa which also means the "Rendering..." message now appears instantly, even on slow systems!