Event value priority #4858
Event value priority #4858
Conversation
TheLimeGlass
added
bug
|
|
||
| @Override | ||
| public int compareTo(EventValueInfo<?, ?> other) { | ||
| return Integer.compare(Math.abs(priority), Math.abs(other.priority)); |
There was a problem hiding this comment.
Instead of worrying about cleaning numbers here, maybe an IAE if the priority is below 0 during construction
There was a problem hiding this comment.
Priority in the negatives can be used by addons to override Skript's priority as Skript never uses negative numbers.
There was a problem hiding this comment.
If you take the absolute value though, then an addon can't really override Skript can it?
If Skript has a priority of 10 and my addon's is -20, Skript wins after absolute value calculations
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
Co-authored-by: APickledWalrus <apickledwalrus@gmail.com>
There was a problem hiding this comment.
Really just some thoughts/questions
Also see: #4858 (comment)
| private final static List<EventValueInfo<?, ?>> defaultEventValues = new ArrayList<>(); | ||
| private final static List<EventValueInfo<?, ?>> futureEventValues = new ArrayList<>(); | ||
| private final static List<EventValueInfo<?, ?>> pastEventValues = new ArrayList<>(); |
There was a problem hiding this comment.
If these need sorted and can't have duplicates perhaps it would be better to use a TreeSet?
There was a problem hiding this comment.
I already tried that, it wasn't working and I tried everything. The sets ended up having only 4-6 values for some reason. I literally even made a custom class and everything to ensure the comparison wasn't being the issue, even down to the fields of the event value infos being compared, but no matter what I tried it never wanted to work with a tree set. This is also why the equals method there is over the top, that was me debugging why the hell it wasn't working. My only guess is because of the generics.
|
Needs more testing after #5503 has fixed most the issues this in-tales to fix. |
Moderocky
force-pushed
the
master
branch
2 times, most recently
from
3f08853 to
bd134d0
Compare
|
Do you still intend to work on this? |
|
closing due to inactivity |
Description
Adds event value priority so that we can fix conflicts easily.
There was an issue where somehow addons could register duplicated event-values to those as Skript, and have the addons take priority. Adding a priority is a semi fix to the solution.
Adds new register methods to EventValues,
priorityandafterwhich examines existing EventValueInfos.I balanced the priority values to have everything work together.
I added a debug method that displays the sorted values after they're all registered. I think i'm the only one that uses debug verbose.
Target Minecraft Versions: any
Related Issues: There are a bunch of issues indirectly related, this is a tool to help aid them all.