[WIP] Replay system #365

NQNStudios · 2024-06-13T18:29:52Z

This is the big one.

NQNStudios · 2024-06-13T18:32:29Z

src/tools/prefs.win.cpp

Preferences need to be recorded and retrieved by the replay system, or else the replays won't be deterministic in a lot of very unpredictable ways.

On Mac, a snazzy system library is used to manage preferences. But I think we should change things so that Mac handles preferences the same as Windows and Linux, because that way recording and replay can be deterministic on Mac too.

I'd prefer not to change how the Mac stores preferences, and I don't understand how the Mac's preferences system gets in the way of what you want to do.

So, looking through the code it seems like a Mac library handles all of the preference syncing behind the scenes, and I couldn't find an API function that would let me iterate through and serialize all of the preferences. It looked like the Mac API provides a dictionary object where you have to know a key to get it's value.

Which is really, really, weird. So maybe I just missed something.

https://github.com/calref/cboe/blob/master/src/tools/prefs.mac.mm#L73

Under the hood, MacOS is using this NSUserDefaults object to store preferences: https://developer.apple.com/documentation/foundation/nsuserdefaults

Oh. This might be what I was looking for and couldn't find: https://developer.apple.com/documentation/foundation/nsuserdefaults/1415919-dictionaryrepresentation

So maybe I can use that function to write Mac-specific logic that serializes and deserializes the NSUserDefaults, but that seems weird to me, when we could run the same code on all 3 platforms instead.

I also don't know the programming language that prefs.mac.mm is written in, which makes things awkward. Is it Objective-C?

Yes and no. It's Objective-C++ – an unholy fusion of C++ and Objective-C.

You should probably be aware that things other than the core preferences are stored in the MacOS preferences plist. For example, it'll remember the last folder you opened a saved game from. Such things likely shouldn't be persisted in the replay, as they are platform-dependent.

Theoretically, the scenario and character editors could also have replay systems

CelticMinstrel

Looks decent enough as a starting point. The goal of recording the entire application (as opposed to just a game session) seems slightly absurd to me, but I guess we'll see.

src/tools/prefs.win.cpp

proj/xc12/BoE.xcodeproj/project.pbxproj

CelticMinstrel · 2024-06-13T23:27:41Z

src/tools/replay.cpp

+}
+
+void record_action(std::string action_type, std::string inner_text) {
+	Element* root = log_document.FirstChildElement();


This isn't really an issue, but I'm noticing you didn't use the XML printer class that the scenario editor uses to write scenario files. It might not actually be appropriate here, I'm not sure; but in case you simply didn't know about it, I thought I'd mention it.

What's the situation with this? Is it appropriate to use my XML printer class here or not?

src/tools/replay.cpp

src/game/boe.main.cpp

CelticMinstrel · 2024-06-15T01:16:16Z

src/tools/replay.cpp

@@ -83,8 +83,7 @@ std::unique_ptr<Element> pop_next_action(std::string expected_action_type) {
 	Element* clone = next_action->Clone()->ToElement();
 	root->RemoveChild(next_action);

-	std::unique_ptr<Element> ptr { clone };
-	return ptr;
+	return *clone;


If this is what you're doing, do you really need to clone? Couldn't you just replace the clone line by Element clone = *next_action;?

Calling RemoveChild() on next_action was freeing the data. Cloning is necessary to keep it around without leaving it in the document object

Are you saying that Element clone = *next_action does not copy it?

But as it is now, isn't the clone still being leaked?

But as it is now, isn't the clone still being leaked?

when a block does

{ Element next = pop_next_action() }

I thought the block is taking ownership of the data, then when the scope ends, it gets deleted.

Are you saying that Element clone = *next_action does not copy it?

Maybe it would.

For it to not crash, Clone() is required.

Element clone = *(next_action->Clone()->ToElement());

This works. Does that avoid a memory leak?

I think calling Clone() in and of itself might be a memory leak, but I'd have to see the internals of Clone() be sure. The typical internals of a function called clone is essentially return new ClassName(*this), and if that's what it's doing, then this line is certainly a memory leak. But maybe Clone() is actually doing something else. I'm not sure.

I looked into Clone() and it's definitely calling new. Which brings me full-circle to what I was trying to accomplish by returning a unique_ptr: I thought wrapping the pointer returned by Clone() in unique_ptr would make the unique_ptr manage that memory, so when the pointer went out of scope in the calling context of pop_next_action(), it would delete the clone.

I think the document object might still manage the memory of the pointer returned by Clone() though. Because I get a segfault when unique_ptr manages the pointer and frees it.

I looked into Clone() and it's definitely calling new. Which brings me full-circle to what I was trying to accomplish by returning a unique_ptr: I thought wrapping the pointer returned by Clone() in unique_ptr would make the unique_ptr manage that memory, so when the pointer went out of scope in the calling context of pop_next_action(), it would delete the clone.

This is a more or less correct line of reasoning. The only problem is, it's not clear whether deleting the clone is the correct way to dispose of it.

* DRY, standardized window top offset * handle_splash_events() handle multiple events per frame * accurate windows menubar height for multiple rows * Windows filter a resize event triggered by the menubar * windows expand small window to fit menubar * splash screens draw in view rect, not window rect

NQNStudios · 2024-07-03T19:38:49Z

That's annoying -- I wanted to rebase this onto master and it ended up adding commits from master into the PR

EDIT: trying again.

CelticMinstrel · 2024-07-05T00:21:07Z

Kinda looks like you rebased it backwards? If you're on the replay branch, you want to rebase it onto master with git rebase master. You definitely don't want to be on master and rebase onto this branch.

NQNStudios added 6 commits June 13, 2024 12:10

record and replay srand() in an action log

e641148

pre-declare action log vars/functions for global use

0d07487

windows and linux record preferences at startup

750d550

don't save preferences when replaying

948981f

add include ticpp.h

4b7fa3d

allow providing base filename for log recording

ac7bc0d

NQNStudios marked this pull request as draft June 13, 2024 18:30

NQNStudios commented Jun 13, 2024

View reviewed changes

NQNStudios added 6 commits June 13, 2024 13:22

Reorganize global replay code

e06474d

Theoretically, the scenario and character editors could also have replay systems

don't double-compile ticpp.cpp

6099045

all scons platforms add src include directories

31d7829

include string and sstream directly

fee62a3

visual studio projects add new files

d27f28a

try adding replay.cpp to xcode projects

66d9faa

CelticMinstrel reviewed Jun 13, 2024

View reviewed changes

NQNStudios added 5 commits June 13, 2024 18:12

refactor without raw pointers

aa93b20

include <memory>

1680467

add cppcodec header library for base64

4dda8a2

action log has_next_action()

c0fc628

WIP encode and decode save files in action logs

a4cc36e

CelticMinstrel reviewed Jun 14, 2024

View reviewed changes

src/tools/replay.cpp Outdated Show resolved Hide resolved

CelticMinstrel reviewed Jun 14, 2024

View reviewed changes

src/game/boe.main.cpp Outdated Show resolved Hide resolved

NQNStudios added 7 commits June 14, 2024 09:13

pop_next_action() return the Element itself

94f1a65

put decoded replay save in tempDir

72b5c38

finish_load_party()

4ecdb91

try adding cppcodec to visual studio include paths

4aab654

fix add cppcodec to visual studio include paths

b7924ea

xcode add cppcodec to search paths

12ae75c

try adding src/tools to xcode header search path

e3eb8a9

CelticMinstrel reviewed Jun 15, 2024

View reviewed changes

NQNStudios added 5 commits July 3, 2024 13:36

Fix fullscreen window sizing on MacOS HighDPI display

8f9c543

More homebrew search paths in Mac Scons

5f48d77

fix calref#368

d5a198f

For now, use raw Element* because it works

43ef19c

NQNStudios force-pushed the replay-system branch 2 times, most recently from bf9900f to 43ef19c Compare July 3, 2024 19:48

NQNStudios mentioned this pull request Jul 3, 2024

[WIP] Replay system rebased #375

Closed

NQNStudios closed this Jul 3, 2024

NQNStudios deleted the replay-system branch September 8, 2024 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Replay system #365

[WIP] Replay system #365

NQNStudios commented Jun 13, 2024

NQNStudios Jun 13, 2024

CelticMinstrel Jun 13, 2024

NQNStudios Jun 13, 2024

NQNStudios Jun 13, 2024

NQNStudios Jun 13, 2024

NQNStudios Jun 13, 2024

NQNStudios Jun 13, 2024 •

edited

Loading

CelticMinstrel Jun 13, 2024 •

edited

Loading

CelticMinstrel left a comment

CelticMinstrel Jun 13, 2024

CelticMinstrel Jul 5, 2024

CelticMinstrel Jun 15, 2024

NQNStudios Jun 15, 2024

CelticMinstrel Jun 15, 2024

NQNStudios Jun 15, 2024 •

edited

Loading

NQNStudios Jun 15, 2024

NQNStudios Jun 15, 2024

CelticMinstrel Jun 16, 2024

NQNStudios Jun 18, 2024 •

edited

Loading

NQNStudios Jun 18, 2024

CelticMinstrel Jun 19, 2024

NQNStudios commented Jul 3, 2024 •

edited

Loading

CelticMinstrel commented Jul 5, 2024

[WIP] Replay system #365

[WIP] Replay system #365

Conversation

NQNStudios commented Jun 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NQNStudios Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

CelticMinstrel Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

CelticMinstrel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NQNStudios Jun 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NQNStudios Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NQNStudios commented Jul 3, 2024 • edited Loading

CelticMinstrel commented Jul 5, 2024

NQNStudios Jun 13, 2024 •

edited

Loading

CelticMinstrel Jun 13, 2024 •

edited

Loading

NQNStudios Jun 15, 2024 •

edited

Loading

NQNStudios Jun 18, 2024 •

edited

Loading

NQNStudios commented Jul 3, 2024 •

edited

Loading