ppy / osu-web

the browser-facing portion of osu!
https://osu.ppy.sh
GNU Affero General Public License v3.0
983 stars 386 forks source link

BanchoBot nomination resets bugged #8006

Closed NoffyNoffeh closed 3 years ago

NoffyNoffeh commented 3 years ago

When a beatmap is reset on the mapper updating the map, this has been working incorrectly. Instead of showing 0 nominations on the modding page, it still shows 1/2, while saying bancho reset nominations and removing the "nominated by" text.

1/2 nominations after bancho reset image

2/2 nominations when a full BN nominated after bancho reset image

This is causing issues besides just visually, because that 1/2 nomination is registering in the system as a non-full BN nomination, making probation BNs unable to nominate maps at 1/2 after being popped by BanchoBot. The only way to fix this is for full BN or NAT to manually pop the map.

Example image from a probation BN unable to nominate a bugged map:

image

Further examples of maps affected

https://osu.ppy.sh/beatmapsets/1070100/discussion/-/generalAll

Was at 1/2 despite being reset by BanchoBot.

https://osu.ppy.sh/beatmapsets/1486133/discussion/-/events

Happened to this map several days ago, and it's showing 3/2 nominations with only 2 nominators while in qualified. This is the oldest instance reported to me, having happened 5 days ago.

Edit: https://osu.ppy.sh/beatmapsets/975467/discussion/-/events Went to qualified with only one nomination after bancho reset

Considering the issues it has for probation BNs this bug should be fairly high priority to get fixed.

nanaya commented 3 years ago

Uh, I forgot about that. @peppy resetting nomination now also involves setting reset, reset_user_id, and reset_at fields of active nominations in beatmapset_nominations table and adding additional reset events in beatmapset_events for each active nomination users:

beatmapset_id: ...
user_id: nominator user id
type: nomination_reset_received
comment: {"beatmap_discussion_id":same_as_current_reset,"beatmap_discussion_post_id":null,"source_user_id":3?,"source_user_username":"banchobot?"}

Maybe should just add an endpoint for banchobot to hit to reset a nomination :thinking:

NoffyNoffeh commented 3 years ago

Not sure if totally related but it's now been reported to me that updates on ranked maps also triggers the reset message,

https://osu.ppy.sh/beatmapsets/1294563/discussion/-/generalAll#/2538788

So this should also be double checked when making changes to not trigger the message on ranked maps as it calls attention to the hax update and causes confusion / looks buggy.

notbakaneko commented 3 years ago

an interop endpoint seems better than multiple things trying to do the same thing đź‘€

NoffyNoffeh commented 3 years ago

It looks like the visual part got fixed on some of the maps originally linked but i also got a report about https://osu.ppy.sh/beatmapsets/975467/discussion/-/generalAll which had gone into qualified with only one nomination after reset and was showing as 1/2 before I disqualified it.

Not sure what the difference between this map and the other maps previously showing 2/2 with one nomination and staying in pending was.

peppy commented 3 years ago

@NoffyNoffeh do any remaining issues exist here after the subsequent fixes?

NoffyNoffeh commented 3 years ago

Looks to be working all good after checking through. Thank you!