VoxelBrushManager uses Multimap<Class, String> instead of Map<String, Class> about voxelsniper-reimagined HOT 5 CLOSED

The current implementation mapps a Brush Class like TreeSnipeBrush to multiple so-called handles. To my understanding, these act like aliases and produce the same result when used.

It currently uses the type Multimap<Class<? extends IBrush>, String>.
With that, it mapps e.g.:
TreeSnipeBrush.class => "t", "tree", "treesnipe"

It possibly can be changed to a Map<String, Class<? extends IBrush>>. I think this is what you meant with this issue.
Now the following mapping would take place:
"t" => TreeSnipeBrush.class
"tree" => TreeSnipeBrush.class
"treesnipe" => TreeSnipeBrush.class
therefore, there would be more data stored then needed.

from voxelsniper-reimagined.

That makes sense, The main reason we wanted a HashMap is because it's search performance is roughly O(1) while the search performance for a MultiMap at least how we currently do it is O(n) with n being the amount of entries

from voxelsniper-reimagined.

Maybe to add to @KevinDaGame's explanation:
The main point was that we had to check every entry of the multimap to find the required brush class, while we never really needed to get the names based on a class (except for unit tests, in which case performance is not as important). I think the amount of extra data stored is relatively small and worth the improvement in my opinion.

https://github.com/KevinDaGame/VoxelSniper-Reimagined/blob/master/VoxelSniperCore/src/main/java/com/github/kevindagame/VoxelBrushManager.java#L139-L143

from voxelsniper-reimagined.

than, this could be closed now.

from voxelsniper-reimagined.

not sure why it didn't close automatically, closed manually

from voxelsniper-reimagined.