garbagemule / MobArena

MobArena plugin for Minecraft
GNU General Public License v3.0
195 stars 151 forks source link

Random Class Selection Issue #786

Open covector opened 8 months ago

covector commented 8 months ago

Bug Report

Short Description

Using /ma class random or clicking a sign with the word random should work and give the message "You will get a random class on arena start." Instead, when using /ma class random, it said "There is no class named random." Did some scuffed fix in a fork and it seems to work (more info in Additional Info).

Reproduction Steps

Using /ma class random or clicking a sign with the word random.

Details

Additional Info

  1. In the file PickClassCommand.java, it seems that the logic for checking if a class slug existed is done before that for checking if the slug is "random". The same goes for the function handleSign in the file ArenaListener.java.
  2. After fixing that, the player becomes unable to ready, getting the message "You must first pick a class!". In the ReadyCommand.java, it didn't take into account the arena class being null (in the case of random class). The same goes for the function handleReadyBlock in the file ArenaListener.java.

Two other things I think should be addressed:

  1. Function assignRandomClass in ArenaImpl.java didn't support for class chest.
  2. If a player chooses another kit after running the command /ma class random, the Arena should remove the player from its randoms set.
garbagemule commented 8 months ago

Thanks for the bug report and for taking the time to dig into what's going on :)

It sounds like maybe the random class assignment isn't working correctly. Browsing through the code, it does seem like there is at least one missing null check in the handleSign method, but I can't figure out how long it's been a problem for. Perhaps the random class assignment isn't the most prominent feature. I'll have to take a closer look soon, but if you have a fix ready, feel free to submit a PR for review or hit me up with a patch on Discord.

garbagemule commented 8 months ago

Just tried clicking a class sign with Random on it, and I got this exception:

https://pastebin.com/DMJBsHF8

Definitely a bug. Not sure when it was introduced.

covector commented 8 months ago

For your reference, this is the commit of the fix on my fork for a private server, it is quite scuffed tho: https://github.com/covector/MobArena/commit/1b65ea23b29cd54dc5ac2da119eefaec0d11d4ed # (sorry but please ignore the changing gamemode to adventure line, as it is for another thing)

garbagemule commented 1 month ago

Oh wow, I never reported back on this. Sorry!

I did do some digging back then, and I found out that the bug was introduced June 8, 2012 (!!), so ever since the class limits were introduced, random class selection via signs hasn't worked. To sum up some of my ramblings on Discord about this issue:

I know it's been a long time since you reported this issue, but are you still using random class assignments today? Is it a feature you still care about?