Caeden117 / ChroMapper

GitHub repository for ChroMapper, the Unity-based map editor for Beat Saber.
https://cm.topc.at/dl
GNU General Public License v2.0
302 stars 87 forks source link

Beatmap Model Refactor #430

Closed KivalEvan closed 1 year ago

KivalEvan commented 2 years ago

I generally don't think the current beatmap model is in good state, nor my current solution are great to fix this. I do want to push development on beatmap v3 but the model absolutely needed to be refactored. Sorry CCDV2, big fat merge conflict on this #417

UPDATED : 2023/01/08

What's changed?

+ Complete refactor and update of beatmap model
+ Custom data conversion
+ Use custom key to retrieve specific custom data; this means v2 and v3 specific object can retrieve the same custom data of different key (override or class check may still be required if contain different behaviour)
+ Enforced beatmap version
* White and transition event now works in beatmap v2
* Adjusted event input; adjust brightness for light event and many other tweaks
* Arc and chain copy now moves tail time
* Obstacle visual changed; they may seem taller and below grid, but it is now more accurate to in-game (This may need opinions)
* Rename grid collection classes with "GridContainer" suffix to reduce confusion
* Fixed BPM change MM editor offset dialog box not showing up
* Fixed mirroring when custom data exist

There are potentially more I've missed or forget to mention. Others worth mentioning being custom data now uses the correct custom key; _color for v2 and color for v3. Arcs and Chains now also show custom color.

What's next or could be improved

While my only goal for this is to make beatmap model much more maintainable and understood better, I still believe there is still more to be improved here and certainly is not ready yet. For any other features, I'll do so in another PR, this should only focus on beatmap refactoring. Perhaps I could try to resolve conflict with CCDV2's PR.

Optimise custom data : PARTIALLY COMPLETED

Custom stuff that is used by ChroMapper is parsed and make use of native type than previously converting to type from JSONNode, this theoretically should remove overhead and become more performant. These customs will be converted to JSONNode upon ToJson and Clone and parsed on new construct. There is possible more optimisation or change to be made if this is not yet ideal.

Update Unit Test : NEXT ON THE LIST

Would try to get this if it's a priority too.

Proper container/collection : Undecided

Maybe? Still works as is but does looks kinda ugly

Proper loading : Undecided

:shrug: again