Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to include simple and/or full event history in savegame #2320

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

jrb0001
Copy link
Contributor

@jrb0001 jrb0001 commented Jun 30, 2024

Simple and full event history can be toggled separately. If one option is enabled:

  • Include that history in Dialogic.get_full_state() / Save.save().
  • On Dialogic.load_full_state() / Save.load() replace that history with the history from the savegame.
    • If the savegame doesn't contain that history, clear it.
    • Events appended to the full event history between the clear_game_state() and the load_game_state() are dropped intentionally.
  • On Dialogic.clear(FULL_CLEAR), clear that history.

The changes to the text event are needed to avoid duplicating the last entry in the simple history after a save+load. This also makes the text show up immediately after loading instead of the normal typing style.

Feel free to suggest a better option name or tooltip.

@Jowan-Spooner
Copy link
Collaborator

This looks good to me on the surface, will try to take a closer look the coming days.

Adjust some code to follow the gdscript style guide.
@Jowan-Spooner
Copy link
Collaborator

I think this makes sense. I made some changes to follow the gdscript style guide.
I'm not entirely sure whether having these histories get erased on clear() is a good thing, but am willing to merge this and potentially adjust the behaviour in future PR's.

This incidentally fixes #1316

@Jowan-Spooner Jowan-Spooner merged commit 39ba323 into dialogic-godot:main Jul 2, 2024
2 checks passed
@Jowan-Spooner
Copy link
Collaborator

Thanks and congrats on your first PR!

@jrb0001
Copy link
Contributor Author

jrb0001 commented Jul 2, 2024

Thanks for the cleanup!

I thought about adding another option for the clear behavior but decided against it in the end. Enabling the save/load behavior more or less makes it part of the game state, like variables. So I wanted to mirror their behavior for consistency.

I can only see the use case where a game has many relatively short and repeatable timelines and uses dialogic variables as temporaries while keeping the real state outside dialogic. And unless such a game supports saving inside such dialogue, it wouldn't use the dialogic save system anyway so it has no reason to enable the option.

I am not against adding a separate clear option, but I also didn't want to clutter the settings tab with something that might never be used (independently).

@jrb0001 jrb0001 deleted the history-save branch July 3, 2024 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants