Repository navigation
Checkpoint: Add tile change persistence - #2877
NataliaOspinal wants to merge 11 commits into
Conversation
|
Play this branch at https://play.threadbare.game/branches/NataliaOspinal/main/. (This launches the game from the start, not directly at the change(s) in this pull request.) |
|
|
||
| # Store everything including position, paths, and consumed tiles in a single dictionary | ||
| # bound strictly to each enemy to prevent synchronization issues. | ||
| static var saved_enemy_states: Dictionary = {} |
There was a problem hiding this comment.
As commented on #1208 (comment) I don't think static variables are an appropriate approach.
Gotta fuse the new code with the og checkpoint in a way that doesn't affect (?) existing ones
|
@NataliaOspinal I see you have added new changes. Let us know when it's ready for review again. |
|
Hello! The code is ready for review, I gave notice on the Issue itself a few days ago, but just in case I'm giving notice here too! @manuq |
manuq
left a comment
manuq
left a comment
There was a problem hiding this comment.
This is looking good, but I wasn't able to test it! Please provide a working example. You could add one in scenes/dev/level_design/. Check for instance, the scenes/dev/level_design/doors_and_levers/.
There was a problem hiding this comment.
The comment in the ticket "Imagine that the template enemies are void-spreading enemies which chase you, but can't cross the bridges" was hypothetical. Sorry for the confusion. Please revert any changes to guard.gd.
There was a problem hiding this comment.
Okay got it! I created a testing scene inside scenes/dev/level_design/void_persistence and reverted the changes in the guard.gd script! Let me know if more changes are needed!
|
Nice, if I abandon the demo and continue it, I can see it persists: void-persisted.webmAnd it resets properly if I restart the demo quest. |
| if path_walk_behavior: | ||
| data["path_progress"] = path_walk_behavior.get("progress_ratio") |
There was a problem hiding this comment.
PathWalkBehavior doesn't have any progress_ratio property. Am I missing something or is this a leftover?
| if path_walk_behavior and data.has("path_progress"): | ||
| path_walk_behavior.set("progress_ratio", data["path_progress"]) |
There was a problem hiding this comment.
Same, no "progress_ratio" in PathWalkBehavior.
| for listener: Node in get_tree().get_nodes_in_group("persistence_listeners"): | ||
| if listener.has_method("_on_checkpoint_activated") and not activated.is_connected(listener._on_checkpoint_activated): | ||
| activated.connect(listener._on_checkpoint_activated) | ||
| activated.emit(self) |
There was a problem hiding this comment.
Using the nodes in a group for connecting and then immediately calling the signal, makes the signal irrelevant. This could be done by calling the method directly:
get_tree().call_group("persistence_listeners", "_on_checkpoint_activated")
Now, there is a better way of doing this, with a signal and without the grouping. For that, please check how the signal lights_changed in per_scene_state.gd is used to turn on/off lights in the game. Then add a similar signal checkpoint_activated one.
| ## Dictionary to store arbitrary persistent scene facts, such as enemy states and consumed tiles. | ||
| @export var facts: Dictionary = {} |
There was a problem hiding this comment.
Interesting! We already have 2 other generic facts, one global GameState.global.facts, and one per-quest GameState.quest.facts.
For collected buttons, we use those because we don't want the buttons to re-appear when replaying the scene. Now I think for the Void enemy position and consumed tiles, it makes sense to add a new per-scene generic facts. Adding this here will make them reset when the scene changes. Seems to be what we want.
|
@NataliaOspinal this is looking nice! If you can, in addition to the request above, please:
|
|
@NataliaOspinal do you know if you will continue working on this? |
|
I'm going to close this for now. @NataliaOspinal feel free to reopen! |
|
I think I got it now! I edited the dev level and implemented the variables in PathWalkBehavior, plus changed the get_tree().call_group("persistence_listeners", "_on_checkpoint_activated") to a signal in per_scene_state! I did change the scripts a couple of days ago but I was struggling to make the void enemies chase the player in the dev level so it can be easier to check the changes. I'll push the changes now, let me know if it needs more editing! |
Should I create a new PR with a reference to this one? It doesn't let me re-open |
Interesting. I just reopened it for you. |
|
Hi! @manuq Just in case I'm letting you know I pushed the new changes last time, the day the issue was re-opened. Let me know if there's more editing to be done! Thank you for the helpful reviews! 😄 |
3 participants
Whenever an enemy changes part of the tilemap as the VoidSpreadingEnemy does, if the player reaches a checkpoint and dies, the scene resets to its original state instead of keeping the changes that we'd like to keep in previous traveled zones for storytelling or possible gameplay mechanics.
To fix this, I created a new Checkpoint child class (I wasn't sure I could edit the original, but if that's the idea I could just add a checkbox for persistent enemies and the respective exports to incorporate the code to the original) that effectively tracks with both _process and _physics_process for accuracy of tile changes, checking the respective paths and its enemy states.
Note: I've included (as part of the code) a way to track the changes of a possible void-spreading guard, since that was the idea shown in the screenshots of the issue!
Resolves: #1208