Add Champ mini game 2 #1805
Add Champ mini game 2 #1805
Conversation
|
This branch was open for a while (during my winter break), so it fell behind the main branch. In an attempt to rebase, there were many conflict issues with files that I did not touch on this branch (and ensured were nowhere in my git history before committing). I decided to reset to before the rebase and am pushing only my changes, as I believe that shouldn't overwrite anything outside the scope of the Champ SQ. If this is incorrect and there is anything I can do to fix it, please let me know! |
|
Play this branch at https://play.threadbare.game/branches/endlessm/champ-mini-game-two. (This launches the game from the start, not directly at the change(s) in this pull request.) |
This probably means that you are rebasing onto the wrong target. Can you share the exact command you are using here? I tested the following locally with the branch from this PR: git switch champ-mini-game-two
git fetch origin
git rebase origin/mainAnd the branch rebased cleanly. It's not a problem that the branch is not rebased. There are no conflicts so it can be merged cleanly either way. |
I did essentially the same set of commands. However, I did |
| [node name="Sprite2D" type="Sprite2D" parent="."] | ||
| visible = false | ||
| rotation = 1.5707964 | ||
| texture = ExtResource("2_rwx2y") |
There was a problem hiding this comment.
I think this hidden Sprite2D is unused and can be removed.
This rock is intended to detect the player (and knock them back) only when the rock is submerged. Previously the Area2D would always detect the player but skip the signal emission if the rock is not submerged; and fake a signal emission in _physics_process() if the rock is submerged and overlapping a body. Instead, sync the `Area2D.monitoring` property to the `submerged` property. Also reduce the number of places that the animation is updated.
| func _physics_process(delta: float) -> void: | ||
| if area_2d.get_overlapping_bodies() != [] and submerged: | ||
| _on_area_2d_body_entered(Node2D.new()) |
There was a problem hiding this comment.
It took me a while to figure out what this is meant to do. I think what you are trying to deal with is this sequence of events:
- Rock is not submerged
- Player walks onto rock
- _on_area_2d_body_entered() is called, but because the rock is above water
water_enteredshould not be emitted
- _on_area_2d_body_entered() is called, but because the rock is above water
- Rock becomes submerged
At this point you want to knock the player back, so in essence you fake a body_entered signal being emitted.
I think there is a simpler way to do this. I'll push a patch. Actually the result is 1 line longer but I think it's clearer & more elegant.
| [sub_resource type="SpriteFrames" id="SpriteFrames_27hsp"] | ||
| resource_local_to_scene = true | ||
| animations = [{ | ||
| "frames": [{ | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_upwns") | ||
| }], | ||
| "loop": true, | ||
| "name": &"default", | ||
| "speed": 10.0 | ||
| }, { | ||
| "frames": [{ | ||
| "duration": 1.0, | ||
| "texture": ExtResource("10_wr0r8") | ||
| }], | ||
| "loop": true, | ||
| "name": &"dry", | ||
| "speed": 10.0 | ||
| }, { | ||
| "frames": [{ | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_riqxt") | ||
| }, { | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_rsmxy") | ||
| }, { | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_mfero") | ||
| }, { | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_rsmxy") | ||
| }], | ||
| "loop": true, | ||
| "name": &"struck", | ||
| "speed": 10.0 | ||
| }] |
There was a problem hiding this comment.
This SpriteFrames resource is duplicated for each rock. Please save it to a file and assign that to each rock.
In fact you already have such a saved resource. The problem is probably that you have set resource_local_to_scene on the resource. This means that the resource will get duplicated each time a scene that uses it is instantiated. There are times when this is useful - for example, if a scene has a CollisionShape that gets modified at runtime, and you have multiple instances of that scene whose collision shapes are independent. I fixed a bug like this recently in another project - endlessm/moddable-platformer@6a00fff.
But in this case the thing to do is to unset that flag and share the resource... oh, I see! The problem is that you actually do modify the resource in champ_sequence_puzzle_object.gd. I don't think that's the right approach but we can do better even without changing that:
- Duplicate the
sequence_puzzle_object.tscnaschamp_rock.tscn. - Change the root node on that scene to use
champ_sequence_puzzle_object.gd(rather than overriding each instance of it inchamp_sequence_puzzle.tscn - Set the
champ_object.tresas its SpriteFrames - Replace the rocks with instances of that scene
Now resource_local_to_scene will behave as desired.
I'll cook a quick patch to make this change.
There was a problem hiding this comment.
This doesn't work either because the sprite_frames property is on the scene root node so is duplicated when the scene is instanced. OK, I'll work around that!
| position = Vector2(-526, -360) | ||
| anchor_mode = 0 |
There was a problem hiding this comment.
I think you should unset these properties because they make it harder to reason about the camera motion. More on this story in the code that moves the camera around...
| position = Vector2(-526, -360) | |
| anchor_mode = 0 |
| await get_tree().create_timer(CAMERA_TIMEOUT_INTERVAL).timeout | ||
| camera.offset.y = camera.offset.y - CAMERA_OFFSET_Y | ||
| camera.zoom.y = camera.zoom.y - CAMERA_ZOOM_SCALAR | ||
| camera.zoom.x = camera.zoom.x - CAMERA_ZOOM_SCALAR |
There was a problem hiding this comment.
Adjusting the camera during the demonstration is a really nice touch! I designed around this in stealth_sequence_combo by having each sequence only use the rocks that are visible when the player is standing at that rock, but it was annoying.
However I think you could consider doing this a bit differently:
- What you are hand-rolling here is effectively a
Tweenhttps://docs.godotengine.org/en/stable/classes/class_tween.html which smoothly animates 1 or more properties with a given curve, duration, and target value. - But Camera2D already knows how to smoothly move itself between positions! I did a little testing and you don't need to adjust the zoom - if you temporarily move the camera to the position of the middle rock of the middle row of the second puzzle, all rocks are in frame. Something like this:
| camera.zoom.x = camera.zoom.x - CAMERA_ZOOM_SCALAR | |
| camera.global_position = $Objects/Middle8.global_position |
(assuming you have taken my other suggestion of resetting the camera's anchor mode and position).
Then you can put it back in _on_hint_sign_hint_sequence_finished with camera.position = Vector2.ZERO.
(The camera still moves with the player so this relies on the fact that the player can't move during the demonstration. You could avoid this theoretical problem by having a second camera that is only used during the demonstration.)
| [sub_resource type="SpriteFrames" id="SpriteFrames_27hsp"] | ||
| resource_local_to_scene = true | ||
| animations = [{ | ||
| "frames": [{ | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_upwns") | ||
| }], | ||
| "loop": true, | ||
| "name": &"default", | ||
| "speed": 10.0 | ||
| }, { | ||
| "frames": [{ | ||
| "duration": 1.0, | ||
| "texture": ExtResource("10_wr0r8") | ||
| }], | ||
| "loop": true, | ||
| "name": &"dry", | ||
| "speed": 10.0 | ||
| }, { | ||
| "frames": [{ | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_riqxt") | ||
| }, { | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_rsmxy") | ||
| }, { | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_mfero") | ||
| }, { | ||
| "duration": 1.0, | ||
| "texture": SubResource("AtlasTexture_rsmxy") | ||
| }], | ||
| "loop": true, | ||
| "name": &"struck", | ||
| "speed": 10.0 | ||
| }] |
There was a problem hiding this comment.
This doesn't work either because the sprite_frames property is on the scene root node so is duplicated when the scene is instanced. OK, I'll work around that!
There was a problem hiding this comment.
At this point, I'm not sure that re-using (and subclassing) the scenes & scripts from the vanilla puzzle is really helping all that much. sequence_puzzle.gd is only 140 lines and this subclass overrides almost all of it!
TBH I have never been all that happy with how tightly coupled this puzzle is. It was I who wrote it, and who made it "generic", but the second user of it - Stella - ended up working around the design forced by each object being a separate scene, when the Stella instance has one turtle with coloured segments of its shell.
I have the feeling that this level would be simpler & less error-prone if you duplicated the scenes & scripts, deleted the generic stuff that is no longer needed, and made the necessary changes, e.g. having the rocks go into a "solved" state that lets you walk over them.
| func _physics_process(delta: float) -> void: | ||
| if player.position.x > SECOND_SEQ_BOUND: | ||
| for i in range(ROCK_WIDTH * first_seq_solve_index, ROCK_WIDTH * (first_seq_solve_index + 1)): | ||
| objs[i].collision.disabled = false |
There was a problem hiding this comment.
This is a lot of work to being doing on every frame. You should instead have an Area2D on the middle island that detects the player, and makes whatever change you want to make. This also avoids you having magic numbers like SECOND_SEQ_BOUND which have to be kept in sync with the level design.
| ## Function to emit a signal to champ specific sequence puzzle script so path can be reset | ||
| func demonstration_finished() -> void: | ||
| hint_sequence_finished.emit() | ||
| super.demonstration_finished() |
There was a problem hiding this comment.
You don't need to override this and handle it in champ_sequence_puzzle.gd because it is the SequencePuzzle script that calls demonstration_finished() (from _on_demonstrate_sequence). Override that function and do the necessary work from _on_hint_sign_hint_sequence_finished there.
| # Ensure the second hint sign camera movement does not cut off a step | ||
| collision_shape.scale = COLLISION_SCALE | ||
| collision_shape.position.y = collision_shape.position.y - COLLISION_OFFSET |
There was a problem hiding this comment.
I don't understand why you would need to change the collision shape of the sign in order to affect the camera movement.
But if you need to change the collision shape: duplicate the scene and change it there, rather than doing it in code.
With the two changes proposed here you can drop this custom script.
Previously, sequence_puzzle_object.tscn was instanced in the puzzle scene with the script overridden on each instance. The overridden script adjusted the collision shape. Instead, duplicate the scene as champ_sequence_puzzle_object.tscn. Set the alternative collision shape there. Remove resource_local_to_scene from the SpriteFrames - this causes it to be duplicated in champ_sequence_puzzle.tcsn. The reason this is necessary at all is because the script modifies the SpriteFrames at runtime, which I think should be changed later, but for now duplicate the resource in code and only if not running in the editor. Replace all instances in champ_sequence_puzzle.tscn with this new scene.
Instead override the relevant functions to be able to change the animation used in the idle state.
wjt
left a comment
wjt
left a comment
There was a problem hiding this comment.
I pushed some patches that address some of my comments. If you're happy with them feel free to merge and we can iterate on the other points!
|
As a player: I really like the concept. Failure has a consequence but it's only a minor setback. We (or more likely, the VRmont cup entrants!) might consider how they might add more pressure or consequence here - should the puzzle be randomized, and after 3 errors you respawn and the puzzle is re-randomized? should something be slowly chasing you? Or maybe it's a problem-solving thing - might there be have a hint sign + demo for the first challenge; then just a hint sign (no demo) for the second one; and then a third one where you have to use some environmental cues to solve it? One thing I find conceptually confusing is as follows. Consider the first sequence of rocks: I'm using Now I have to hit the middle rock of the second column; and so on. But suppose I get it wrong. In that case the big rock goes back under the water. The first rock stays raised: So it looks like I can walk across the rock, but actually I have to kick it again to raise the big rock again. Also what's a bit odd is that when I am in this state: If I accidentally kick the top-left rock again, I am once again washed back. It feels a bit weird. I wonder if you need a third visible state for the "solved but needs to be kicked again" state; and to disable interacting with the first column once the correct first rock has been kicked, until the player makes a mistake. It also does seem a little strange that you can't walk back the way you came, even though visually it looks like you should be able to. I think you could change the logic there: rather than re-enabling collisions and blocking the player from going back, you could instead disable interacting with the solved puzzle. Just a thought! |
The sequence minigame has Evan continuing his journey along the shore of Lake Champlain. He needs to cross part of the lake via some stepping stones that are submerged underwater. Only certain rocks will allow him to stand on them without fully submerging, forcing Evan to start again!
This minigame involved the extension of scripts for sequence puzzle logic, hint signs, and sequence puzzle objects, as well as the addition of a new champ_long_rock script.
Assets:
champ_object.png by @felixwalberg
champ_sign.png by @felixwalberg
champ_animated_water_rock.png by @felixwalberg
champ_dry_rock.png by @felixwalberg
champ_long_rock.png by @felixwalberg
champ_long_water_rock.png by @felixwalberg
champ_wet_rock.png by @felixwalberg
water_rocks_sequence.png by @felixwalberg
Engineering:
Sequence Puzzle Level by @felixwalberg
Game Design:
Core Gameplay & Mechanics by @felixwalberg
Progression & Motivation by @felixwalberg