Skip to content

Conversation

@mzxrules
Copy link
Contributor

@mzxrules mzxrules commented May 3, 2025

Contributions made in this pr are licensed under CC0

This pr makes it so that the SceneID enumeration now stores the values for scenes that are missing in some versions.

Copy link
Collaborator

@Dragorn421 Dragorn421 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the current way the deleted scenes' enum values are handled

@mzxrules
Copy link
Contributor Author

mzxrules commented May 4, 2025

inconsistent and unshifting?

@Dragorn421
Copy link
Collaborator

The current way is simpler, not having to mess with the scene table .h
The values don't need to shift for any purpose (this was discussed back then), but if you really wanted to it'd be simpler to just adjust the definition of the additional debug SCENE_ macros

@mzxrules
Copy link
Contributor Author

mzxrules commented May 4, 2025

i don't understand.

  • If you add scenes you have to touch scene_table.h
  • If you subtract scenes you have to touch scene_table.h
  • If you reorder scenes you have to touch scene_table.h

The difference now is that if you want to use a debug scene like the test map in a non debug setting, you no longer have to update z64save.

Comment on lines +118 to +122
#if !DEBUG_ASSETS && defined(DEFINE_SCENE_ENUM)
DEFINE_SCENE_TABLE_END()
#endif

#if DEBUG_ASSETS || defined(DEFINE_SCENE_ENUM)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This #if logic makes the table unnecessarily hard to read, I'd prefer if we just had DEFINE_SCENE_DEBUG and DEFINE_SCENE_DELETED and defined them at all include sites to keep the table itself easier to read

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how you would implement DEFINE_SCENE_DEBUG in a sane way. The debug scenes are not dummied into the scene table in release builds, so a custom mod that mixed DEFINE_SCENE and DEFINE_SCENE_DEBUG declarations could desync the enumeration.

@Dragorn421 Dragorn421 added the Waiting for author There are requested changes that have not been addressed label Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Waiting for author There are requested changes that have not been addressed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants