-
Notifications
You must be signed in to change notification settings - Fork 260
InkCombatRound6: Implement Sequential Filling Mechanic #1865
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
base: main
Are you sure you want to change the base?
Conversation
- Added the 'barrel_glow' folder containing the sprite sheet and animation resource. - Added a new AnimatedSprite2D node named 'BarrelGlow' to the FillingBarrel scene. - Configured the 'glowing' animation and set the correct offset to align with the barrel. feat: #1847
- Set the Y offset to -21 to align correctly with the barrel sprite. - Set the initial visibility to false so it remains hidden until activated. fix: #1847
- Added new component to handle sequential unlocking logic. - Implemented shuffle and locked state management. feat: #1847
- Updated 'filling_barrel.gd' to ignore inputs when locked. - Added visual feedback handling dimmed color for locked state. feat: #1847
- Added 'BarrelUnlockSequence' node to the level scene. - Configured barrels to unlock one by one in random order. feat: #1847
|
Play this branch at https://play.threadbare.game/branches/endlessm/issue_1847. (This launches the game from the start, not directly at the change(s) in this pull request.) |
|
I pushed a conflict resolution. Without reviewing the code in detail: this level is now extremely hard because you have to evade the Mothsache and also focus on one specific barrel at a time! (Don't feel the need to change this, it's just an observation before we review the logic.) |
|
You’re absolutely right, @wjt. Originally, Level 6 was focused only on the sequential barrel mechanic, but later Mothsache was added to test it, which significantly increased the difficulty. In fact, I had an idea that I didn’t get a chance to share before the internship ended. I’ll open a discussion and share the idea I had in mind with you. |
|
I've just created the discussion regarding the difficulty curve and intermediate levels. You can find it here: #1881 |
wjt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice addition to the filling-game mechanic - I think it will make it possible to design more interesting puzzles.
| @export var color: Color: | ||
| set = _set_color | ||
|
|
||
| var is_locked: bool = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please document this field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also make the set_locked_state function a setter for this field:
| var is_locked: bool = false | |
| var is_locked: bool = false: | |
| set = set_is_locked |
(I suggest renaming the method below.)
| barrel_glow.visible = false | ||
|
|
||
|
|
||
| func set_locked_state(locked: bool) -> void: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| func set_locked_state(locked: bool) -> void: | |
| func set_is_locked(locked: bool) -> void: |
| if barrel_glow: | ||
| barrel_glow.visible = false | ||
| else: | ||
| modulate = Color(1, 1, 1, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| modulate = Color(1, 1, 1, 1) | |
| modulate = Color.WHITE |
|
|
||
| func _ready() -> void: | ||
| if auto_start: | ||
| call_deferred("start_sequence") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| call_deferred("start_sequence") | |
| start_sequence.call_deferred() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why do you need to defer this?
| randomize() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| randomize() |
Since Godot 4.0, the random seed is automatically set to a random value when the project starts. This means you don't need to call randomize() in _ready() anymore to ensure that results are random across project runs. However, you can still use randomize() if you want to use a specific seed number, or generate it using a different method.
| var target: FillingBarrel = barrels[current_target_index] | ||
|
|
||
| if is_instance_valid(target): | ||
| target.set_locked_state(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you add the set = ... syntax I suggest above, then this can be:
| target.set_locked_state(false) | |
| target.is_locked = false |
| push_warning("BarrelUnlockSequence: No barrels assigned.") | ||
| return | ||
|
|
||
| barrels.shuffle() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the order should necessarily be shuffled. The order of the barrels is part of the level design - for example, you could imagine designing a level where it would be impossible to get across a bridge until the first barrel is filled.
Another way of looking at this is: if the order is random each time the level is played, then why do I (the level designer) have to manually assign the barrels to this node, rather than this component discovering the barrels in the scene by itself?
| var filling_barrels: Array = get_tree().get_nodes_in_group("filling_barrels") |
Some ideas:
- Remove the
shuffle()call - Keep it, but add an
@export var randomize_barrel_order: boolvariable (adjusting the default value to taste :) ) - If you think the mechanic is best enjoyed with a random order: keep the shuffle, but remove the
@exportfromvar barrelsand instead populate that array in_readyin the same way thatfill_game_logic.gddoes
| if current_target_index < barrels.size(): | ||
| var target: FillingBarrel = barrels[current_target_index] | ||
|
|
||
| if is_instance_valid(target): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm always a bit suspicious of is_instance_valid calls because I think they tend to mask bugs. In what situation would you expect a barrel to be freed from the scene before it has been enabled? Or is there another case I have not considered which would cause an element of the array to be freed?
| is_locked = locked | ||
|
|
||
| if is_locked: | ||
| modulate = Color(0.5, 0.5, 0.5, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps define a const LOCKED_COLOR := Color(0.5, 0.5, 0.5, 1) constant for this?
| animated_sprite_2d.animation = FILLING_NAME_ANIMATION | ||
| animated_sprite_2d.frame = 0 | ||
|
|
||
| if barrel_glow: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have this line above:
@onready var barrel_glow: AnimatedSprite2D = $BarrelGlow$x is a shorthand for get_node("x"). docs.
get_node() raises an error if the node does not exist.
You should do one of the following two things:
- In the
@onready ...line, replace$BarrelGlowwithget_node_or_null("BarrelGlow"), which does not raise an error if the node does not exist https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-get-node-or-null - Remove all the
if barrel_glow:checks - by using$BarrelGlowwe are requiring that the scene that this script is attached to has a node of that name.
(You may ask, why does _set_sprite_frames have a similar if animated_sprite_2d: check? Good question! One answer is: setters can run before a node is ready, so @onready variables may not have been populated yet. But then you might say: but doesn't that setter check if not is_node_ready(): first? Yes it does. The if animated_sprite_2d: check is redundant.)
This PR implements the Sequential Combo mechanic for Level 6. Barrels must now be filled in a specific, randomized order.
BarrelUnlockSequencecomponent:🎮 Play Ink Drinker Level 6
Play Ink Drinker Level 6
resolves #1847