Enhancement: Room/Scene Timers #2478

stratomaster64 · 2023-02-15T02:15:20Z

(Attempts to) resolve #2459.

Build Artifacts

accounted for null playstate

stratomaster64 · 2023-02-17T03:37:09Z

Checklist for remaining items before this can be considered ready for review:

~~Allowing the scene/room timers to run when paused? - current behavior is that both timers stop, same as playTimer, when the game is paused.~~ Would require being aware of the amount of time being paused while in the scene, which uses a different framerate divisor than during gameplay.
Allowing the user to have less granular timestamps shown - current behavior creates a timestamp everytime a new room is entered (which includes loading a new scene as well)
Pausing timers when entering a scene only to play a cutscene - for example, when starting a vanilla game, timestamps are created for Link's House > Hyrule Field > Kokiri Forest > Link's House before gaining control.
Mapping of scenes to names - current behavior is exposing the scene number to the user, as opposed to the familiar name (Scene 80 instead of Hyrule Field).
Cleanup - Not actually required.

stratomaster64 · 2023-02-28T05:13:25Z

Opening for review to get more eyes/playtesting on this.

Archez · 2023-03-03T15:51:53Z

soh/soh/SaveManager.cpp

-        SaveManager::Instance->LoadArray("timestamps", ARRAY_COUNT(gSaveContext.sohStats.timestamp), [](size_t i) {
-            SaveManager::Instance->LoadData("", gSaveContext.sohStats.timestamp[i]);
+        SaveManager::Instance->LoadArray("itemTimestamps", ARRAY_COUNT(gSaveContext.sohStats.itemTimestamp), [](size_t i) {
+            SaveManager::Instance->LoadData("", gSaveContext.sohStats.itemTimestamp[i]);
        });


So this change means that old timestamps will not be loaded into the new struct properly. Normally when a json change happens, we have to introduce a new LoadBase vX function. However this is non-critical information that will not break in-game functionality.

Curious others thought about this. do we want to add another base func and support the old timestamps to be loaded, or are we fine with old timestamps getting lost.

My gut feeling is I'm fine with losing the old timestamps. I think this is also probably necessary since the old timestamps included more than just "item" timestamps, so they probably wouldn't be able to be brought over correctly anyways without some strange parsing.

I agree, losing timestamps if you load a save from an old build isn't a big enough problem to warrant building out migrations

stratomaster64 · 2023-03-11T23:55:23Z

@briaguya-ai sorry for the ping, just making sure this didn't fall off the radar re: review

briaguya-ai

overall this is looking great! left a few comments, some that are small and can probably be addressed quite easily. some that feel out of scope for this PR but came to mind when i was looking through it.

shouldn't take long to address those and resolve merge conflicts, once that's done this should be good to go!

briaguya-ai · 2023-04-02T09:45:34Z

soh/soh/Enhancements/gameplaystats.cpp

 }

+const std::vector<std::string> sceneMappings = {


this feels similar to sceneNames in util.cpp but it's different enough that i can understand having it here

would be nice to consolidate stuff like this at some point but this is far from the only example of duplicated strings throughout the codebase, definitely not something that should block merging this

Honestly did not know that utils.cpp existed, but I think sceneMappings does have some more user-friendly names. Definitely not a bad idea to combine the two containers

soh/soh/Enhancements/gameplaystats.cpp

briaguya-ai · 2023-04-02T10:11:43Z

soh/src/code/z_play.c

+                      gSaveContext.sohStats.sceneTimer++;
+                      gSaveContext.sohStats.roomTimer++;


could be cool to refactor to use hooks here, but since we're already using playTimer without them that feels out of scope for this PR. probably worth making a new issue for refactoring stats tracking to use hooks.

briaguya-ai · 2023-04-02T10:15:48Z

soh/src/code/z_room.c

+    u8 idx = gSaveContext.sohStats.tsIdx;
+    gSaveContext.sohStats.sceneTimestamps[idx].scene = gSaveContext.sohStats.sceneNum;
+    gSaveContext.sohStats.sceneTimestamps[idx].room = gSaveContext.sohStats.roomNum;
+    gSaveContext.sohStats.sceneTimestamps[idx].roomTime = gSaveContext.sohStats.roomTimer / 2;
+    gSaveContext.sohStats.sceneTimestamps[idx].isRoom = 
+        gPlayState->sceneNum == gSaveContext.sohStats.sceneTimestamps[idx].scene &&
+        gPlayState->roomCtx.curRoom.num != gSaveContext.sohStats.sceneTimestamps[idx].room;
+    gSaveContext.sohStats.tsIdx++;
+    gSaveContext.sohStats.roomNum = roomCtx->curRoom.num;
+    gSaveContext.sohStats.roomTimer = 0;


this looks pretty much identical to what's happening in z_play.c, what's happening in this method that isn't being covered by the logic there?

z_play.c specifically scene transitions, whereas this snippet handles specifically room transitions. z_play has similar code to handle certain respawning states, such as warping from DMC to DMC or falling in the lava there (which causes a voidout); in some cases, voiding out of a scene will cause the room number to change but not the scene number, so that's for some edge cases.

briaguya-ai

looks like this is good to go!

stratomaster64 added 4 commits February 14, 2023 21:14

Groundwork on scene/room timers; naming changes

19038c0

added to save manager; reworked storing timestamps

05f96ce

actually saved stuff to savemanager;

a519bc0

accounted for null playstate

finally fixed the fucking timers

b832778

stratomaster64 added 3 commits February 17, 2023 01:18

Added scene mapping

667cdb5

Merge branch 'develop' into even-more-stats

a2a6b43

Added CVar for room/scene level; fixed some displays

63d7482

stratomaster64 marked this pull request as ready for review February 28, 2023 05:13

stratomaster64 and others added 6 commits March 2, 2023 00:59

reworked logic

55c91d6

Merge branch 'develop' into even-more-stats

d2cc960

increase name spec for scene timestamps

cc7a6e5

Merge branch 'HarbourMasters:develop' into even-more-stats

6b2d36c

Actually save item timestamps when loading v3 save

2799250

Cleanup

4961408

Archez reviewed Mar 3, 2023

View reviewed changes

stratomaster64 added 3 commits March 13, 2023 14:39

Merge branch 'develop' into even-more-stats

20c7112

fix merge artifact

5298f3c

Merge branch 'develop' into even-more-stats

349ae69

stratomaster64 requested review from Archez and briaguya-ai and removed request for Archez March 16, 2023 01:25

briaguya-ai added the merge conflicts PR has conflicts that need to be resolved before it can be merged label Apr 2, 2023

briaguya-ai reviewed Apr 2, 2023

View reviewed changes

Merge branch 'develop' into even-more-stats

0630621

briaguya-ai removed the merge conflicts PR has conflicts that need to be resolved before it can be merged label Apr 3, 2023

apply suggestions

e30d737

briaguya-ai approved these changes Apr 3, 2023

View reviewed changes

briaguya-ai merged commit ff1d8a9 into HarbourMasters:develop Apr 3, 2023

stratomaster64 deleted the even-more-stats branch April 3, 2023 23:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancement: Room/Scene Timers #2478

Enhancement: Room/Scene Timers #2478

stratomaster64 commented Feb 15, 2023 •

edited by github-actions bot

stratomaster64 commented Feb 17, 2023 •

edited

stratomaster64 commented Feb 28, 2023

Archez Mar 3, 2023

briaguya-ai Mar 3, 2023

stratomaster64 commented Mar 11, 2023

briaguya-ai left a comment

briaguya-ai Apr 2, 2023

stratomaster64 Apr 3, 2023

briaguya-ai Apr 2, 2023

briaguya-ai Apr 2, 2023

stratomaster64 Apr 3, 2023

briaguya-ai left a comment

		gSaveContext.sohStats.sceneTimer++;
		gSaveContext.sohStats.roomTimer++;

Enhancement: Room/Scene Timers #2478

Enhancement: Room/Scene Timers #2478

Conversation

stratomaster64 commented Feb 15, 2023 • edited by github-actions bot

Build Artifacts

stratomaster64 commented Feb 17, 2023 • edited

stratomaster64 commented Feb 28, 2023

Archez Mar 3, 2023

Choose a reason for hiding this comment

briaguya-ai Mar 3, 2023

Choose a reason for hiding this comment

stratomaster64 commented Mar 11, 2023

briaguya-ai left a comment

Choose a reason for hiding this comment

briaguya-ai Apr 2, 2023

Choose a reason for hiding this comment

stratomaster64 Apr 3, 2023

Choose a reason for hiding this comment

briaguya-ai Apr 2, 2023

Choose a reason for hiding this comment

briaguya-ai Apr 2, 2023

Choose a reason for hiding this comment

stratomaster64 Apr 3, 2023

Choose a reason for hiding this comment

briaguya-ai left a comment

Choose a reason for hiding this comment

stratomaster64 commented Feb 15, 2023 •

edited by github-actions bot

stratomaster64 commented Feb 17, 2023 •

edited