Render96 / Render96ex

Fork of https://github.com/sm64-port/sm64-port with additional features.
239 stars 43 forks source link

Render 96 v2.2 DynOs not Saving Model Setting #20

Closed rileychan6 closed 3 years ago

rileychan6 commented 3 years ago

Describe the bug The new low poly models are loaded on each new startup even though I set the model pack in DynOS to Render96. To clarify, it does change the models when I change the setting, but it doesn't save when I close the game. If I restart the game, the models reset to their low poly counterparts. Before the latest update, 2.2, there weren't any low poly options included in Render96, so I am wondering if this is just an issue with DynOS loading model packs or if it's specific to Render96.

To Reproduce Steps to reproduce the behavior:

  1. Load game and enter DynOS
  2. Open Model Packs
  3. Change the Render96 option to "Enabled"
  4. Close the game
  5. Re-open the game

Expected behavior I expected the model setting to save between closing and startup as other DynOS settings do (e.g. the Cheater settings save)

Desktop (please complete the following information):

Note This is not a major issue, since the models can be changed properly after each launch; it's just a minor inconvenience.

enigma9o7 commented 3 years ago

Pretty sure the low poly Mario was there before recent updates, probably you just were overwriting him before, had you ever tried render96ex without replacing Mario before? The original Mario was replaced in this fork a little while ago...

But 100% agree this would be a great feature, I was about to suggest myself but you beat me to it.

PeachyPeachSM64 commented 3 years ago

That's not a bug, that was intended, or at least convenient when I implemented it. Internally, pack folders are stored by index and not by name, meaning that adding more packs might shift the indexes around.

Quist suggested changing the naming of packs: "It might also be a good idea to consider changing how those options are named, [...] it would be better to have it be String("dynos_pack_%s", _Packs[i]) so they are identified by name" Indeed, it could be a good idea, but raises two more problems: First, sorting packs by indexes were convenient for the use of the "Disable all packs" button (a simple for loop through all indexes to set every pack option to disabled) Second, the same kind of loop is used in DynOS_Gfx_UpdateModelData, the function responsible for swapping the models around depending on which pack is enabled and which pack is not

rileychan6 commented 3 years ago

Thank you for the clarification on the issue.

enigma9o7 commented 3 years ago

Just a thought, I wouldn't mind if it only shifted indexes around when I add more packs. As long as it stayed the same between games when nothing changed, that's still an improvement, and might require less thought to implement. If I add more packs probably going to go into model loader settings immediately anyway so not such a big deal..