Make model ID's an enum #863
Conversation
someone2639
requested review from
Reonu,
arthurtilly and
gheskett
as code owners
|
Note on enums in general is fast64 integration will need to be implemented before this can be merged. |
|
what. do you think fast64 uses hardcoded model ids. |
|
It uses defines. How do we know the current implementation is compatible (because it probably isn't). |
|
bro |
|
Test it please. |
gheskett
left a comment
gheskett
left a comment
There was a problem hiding this comment.
This PR won't see approval until fast64 is updated to support this for 3.0.
There was a problem hiding this comment.
Please neaten up the line spacing for geo comments, like how they were previously.
|
|
||
| // actor model IDs | ||
|
|
||
| // first set of actor bins (0x54-0x63) |
There was a problem hiding this comment.
Comment no longer relevant.
Also regarding actor entries, the way this is being laid out now doesn't reuse entries, wasting memory. I don't have a good solution proposed for checking against the largest group entries though (and memory loss is probably not huge), so this will be a non-blocking concern.
| const LevelScript script_func_global_13[] = { | ||
| LOAD_MODEL_FROM_GEO(MODEL_BOWSER, bowser_geo), | ||
| LOAD_MODEL_FROM_GEO(MODEL_BOWSER_BOMB_CHILD_OBJ, bowser_bomb_geo), | ||
| LOAD_MODEL_FROM_GEO(MODEL_BOWSER_BOMB, bowser_bomb_geo), |
| JUMP_LINK(script_func_global_17), | ||
| JUMP_LINK(script_func_vo_sl), | ||
| LOAD_MODEL_FROM_GEO(MODEL_SL_SNOW_TREE, snow_tree_geo), | ||
| LOAD_MODEL_FROM_GEO(MODEL_SNOW_TREE, snow_tree_geo), |
There was a problem hiding this comment.
Shouldn't this go in the actual vanilla objects file?
There was a problem hiding this comment.
Wait no it shouldn't, this is part of common1 (so it should be global then). Don't forget about CCM though!
There was a problem hiding this comment.
Now that I'm looking at it...there's actually a lot of common1 geos unaccounted for here. I still think it makes the most sense to globally load all of them, but I guess technically this is a (mostly insignificant) waste of RAM for most hacks.
Also, this is slightly out of scope probably, but can you make the common1 and group0 geo loads for scripts.h into their own jump scripts? The main level entry script is getting pretty messy, plus a jump script has use cases for non-standard level scripts.
|
Converted to draft until fast64 implementation is addressed |
3 participants
One of those inbetween PR's that will make object streaming:tm: less painful
Scope:
MODEL_ID_COUNTis 511 under this PR sogLoadedGraphNodesisn't even doubled in size by default)MODEL_LEVEL_GEOMETRY_XXID's are no longer shared as a resultModelID8becauseMODEL_ID_COUNTwon't fit by default anymore; renameModelID16to justModelId(Should I just make it 32 bit?)level_main_scripts_entryto prevent crashes (and allow warp pipes to be used in levels by default)