Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random Class Selection Issue #786

Open
covector opened this issue Jan 16, 2024 · 5 comments
Open

Random Class Selection Issue #786

covector opened this issue Jan 16, 2024 · 5 comments
Labels
area/misc Issues that don't fit particularly well into any other areas. type/bug Unintended behavior or fault.

Comments

@covector
Copy link

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

  • MobArena version: 0.107
  • Server version: paper 1.20.1
  • Stacktrace (if applicable):

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
Copy link
Owner

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 garbagemule added type/bug Unintended behavior or fault. area/misc Issues that don't fit particularly well into any other areas. labels Jan 16, 2024
@garbagemule
Copy link
Owner

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
Copy link
Author

For your reference, this is the commit of the fix on my fork for a private server, it is quite scuffed tho:
covector@1b65ea2

(sorry but please ignore the changing gamemode to adventure line, as it is for another thing)

@garbagemule
Copy link
Owner

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:

  • Class limits and random class selection are pretty much incompatible concepts, even moreso when you consider class permissions as well. The easiest example to envision this is 2 classes, each with a limit of 1, and 2 players joining. With truly random selection, sometimes both players would be assigned the same class, thus violating the class limits.
  • We could try to solve the problem with a heuristics-guided random distribution, such that the "random" assignment is done according to a set of rules that optimize for a "complete fit". This is a fun theoretical problem, but I don't think it's straightforward to implement, at least not in an intuitive, readable way.
  • If this bug has persisted for over 12 years without anyone reporting it before now, I'd wager it's a very niche feature that is probably much more reasonable to remove than to fix. We could always reintroduce it (but by rewriting it) if there is enough demand for it. On the other hand, I kinda hate the idea of removing a feature after someone reports an issue with it, because that obviously means they were using it, and probably cared about it at least a little bit.

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?

@Valorless
Copy link

@covector I'm a little tardy to the party, but.
I think I could likely create a feature to the MobArena extension I'm working on, where you're assigned a random class when the arena starts.
But as Mule said, this feature would ignore permissions and limits:

Class limits and random class selection are pretty much incompatible concepts, even morsso when you consider class permissions as well. The easiest example to envision this is 2 classes, each with a limit of 1, and 2 players joining. With truly random selection, sometimes both players would be assigned the same class, thus violating the class limits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/misc Issues that don't fit particularly well into any other areas. type/bug Unintended behavior or fault.
Projects
None yet
Development

No branches or pull requests

3 participants