Skip to content

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.

Sign up for GitHub

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

Open
russell-cj wants to merge 6 commits into
base: main
Choose a base branch
from
Open

InkCombatRound6: Implement Sequential Filling Mechanic #1865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
03608a8
add barrel glow animation assets
russell-cj Jan 24, 2026
566ebec
adjust BarrelGlow offset and initial visibility
russell-cj Jan 24, 2026
4a51929
add BarrelUnlockSequence script
russell-cj Jan 26, 2026
d233f85
implement locking capability in FillingBarrel
russell-cj Jan 26, 2026
1c1ec84
integrate sequential barrel mechanic
russell-cj Jan 26, 2026
70c6083
Merge remote-tracking branch 'origin/main' into issue_1847
wjt Feb 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions scenes/game_elements/props/filling_barrel/components/filling_barrel.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ const FILLING_NAME_ANIMATION: StringName = &"filling"
@export var color: Color:
set = _set_color

var is_locked: bool = false
Copy link
Member

@wjt wjt Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document this field.

Copy link
Member

@wjt wjt Feb 9, 2026 •
edited
Loading

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:

Suggested change
var is_locked: bool = false
var is_locked: bool = false:
set = set_is_locked

(I suggest renaming the method below.)


var _amount: int = 0

@onready var barrel_glow: AnimatedSprite2D = $BarrelGlow
@onready var animated_sprite_2d: AnimatedSprite2D = %AnimatedSprite2D
@onready var animation_player: AnimationPlayer = %AnimationPlayer
@onready var collision_shape_2d: CollisionShape2D = %CollisionShape2D
Expand Down Expand Up @@ -70,10 +73,29 @@ func _ready() -> void:
animated_sprite_2d.animation = FILLING_NAME_ANIMATION
animated_sprite_2d.frame = 0

if barrel_glow:
Copy link
Member

@wjt wjt Feb 9, 2026

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:

  1. In the @onready ... line, replace $BarrelGlow with get_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
  2. Remove all the if barrel_glow: checks - by using $BarrelGlow we 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.)

barrel_glow.visible = false


func set_locked_state(locked: bool) -> void:
Copy link
Member

@wjt wjt Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func set_locked_state(locked: bool) -> void:
func set_is_locked(locked: bool) -> void:

is_locked = locked

if is_locked:
modulate = Color(0.5, 0.5, 0.5, 1)
Copy link
Member

@wjt wjt Feb 9, 2026

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?

if barrel_glow:
barrel_glow.visible = false
else:
modulate = Color(1, 1, 1, 1)
Copy link
Member

@wjt wjt Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
modulate = Color(1, 1, 1, 1)
modulate = Color.WHITE

if barrel_glow:
barrel_glow.visible = true
barrel_glow.play("glowing")


## Increment the amount and play an animation. If completed, also play the completed
## animation and remove this barrel from the current scene.
func increment(by: int = 1) -> void:
if is_locked:
return
if _amount >= needed_amount:
return
animation_player.play(&"increment")
Expand Down
79 changes: 79 additions & 0 deletions scenes/game_elements/props/filling_barrel/filling_barrel.tscn
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,82 @@
[gd_scene format=3 uid="uid://y8ha8abfyap2"]

[ext_resource type="Script" uid="uid://be17wk85qlu28" path="res://scenes/game_elements/props/filling_barrel/components/filling_barrel.gd" id="1_md0e3"]
[ext_resource type="Texture2D" uid="uid://brranfy51i56n" path="res://scenes/quests/lore_quests/quest_003/2_ink_drinker_levels/components/barrel_glow/barrel_glow.png" id="3_8hqam"]
[ext_resource type="SpriteFrames" uid="uid://dlsq0ke41s1yh" path="res://scenes/game_elements/props/filling_barrel/components/filling_barrel_sprite_frames.tres" id="3_o1oxn"]
[ext_resource type="AudioStream" uid="uid://6tgopt072bfq" path="res://scenes/game_elements/props/filling_barrel/components/fill.wav" id="4_6inx0"]
[ext_resource type="AudioStream" uid="uid://s7ne07cx0j72" path="res://scenes/game_elements/props/filling_barrel/components/complete.wav" id="5_3gtj0"]

[sub_resource type="AtlasTexture" id="AtlasTexture_ho6qg"]
atlas = ExtResource("3_8hqam")
region = Rect2(0, 0, 128, 128)

[sub_resource type="AtlasTexture" id="AtlasTexture_ig56r"]
atlas = ExtResource("3_8hqam")
region = Rect2(128, 0, 128, 128)

[sub_resource type="AtlasTexture" id="AtlasTexture_ompl8"]
atlas = ExtResource("3_8hqam")
region = Rect2(256, 0, 128, 128)

[sub_resource type="AtlasTexture" id="AtlasTexture_0a20k"]
atlas = ExtResource("3_8hqam")
region = Rect2(384, 0, 128, 128)

[sub_resource type="AtlasTexture" id="AtlasTexture_2evyj"]
atlas = ExtResource("3_8hqam")
region = Rect2(512, 0, 128, 128)

[sub_resource type="AtlasTexture" id="AtlasTexture_i2u8c"]
atlas = ExtResource("3_8hqam")
region = Rect2(640, 0, 128, 128)

[sub_resource type="AtlasTexture" id="AtlasTexture_fc6b6"]
atlas = ExtResource("3_8hqam")
region = Rect2(768, 0, 128, 128)

[sub_resource type="AtlasTexture" id="AtlasTexture_haw5m"]
atlas = ExtResource("3_8hqam")
region = Rect2(896, 0, 128, 128)

[sub_resource type="AtlasTexture" id="AtlasTexture_j04m1"]
atlas = ExtResource("3_8hqam")
region = Rect2(1024, 0, 128, 128)

[sub_resource type="SpriteFrames" id="SpriteFrames_ftrfy"]
animations = [{
"frames": [{
"duration": 1.0,
"texture": SubResource("AtlasTexture_ho6qg")
}, {
"duration": 1.0,
"texture": SubResource("AtlasTexture_ig56r")
}, {
"duration": 1.0,
"texture": SubResource("AtlasTexture_ompl8")
}, {
"duration": 1.0,
"texture": SubResource("AtlasTexture_0a20k")
}, {
"duration": 1.0,
"texture": SubResource("AtlasTexture_2evyj")
}, {
"duration": 1.0,
"texture": SubResource("AtlasTexture_i2u8c")
}, {
"duration": 1.0,
"texture": SubResource("AtlasTexture_fc6b6")
}, {
"duration": 1.0,
"texture": SubResource("AtlasTexture_haw5m")
}, {
"duration": 1.0,
"texture": SubResource("AtlasTexture_j04m1")
}],
"loop": true,
"name": &"glowing",
"speed": 10.0
}]

[sub_resource type="RectangleShape2D" id="RectangleShape2D_8tegq"]
size = Vector2(52, 52)

Expand Down Expand Up @@ -166,6 +238,13 @@ position = Vector2(0, -21)
sprite_frames = ExtResource("3_o1oxn")
animation = &"filling"

[node name="BarrelGlow" type="AnimatedSprite2D" parent="."]
visible = false
sprite_frames = SubResource("SpriteFrames_ftrfy")
animation = &"glowing"
frame_progress = 0.55175865
offset = Vector2(0, -21)

[node name="HitBox" type="StaticBody2D" parent="." unique_id=1847617565]
unique_name_in_owner = true
position = Vector2(2, -18)
Expand Down
3 changes: 3 additions & 0 deletions ...re_quests/quest_003/2_ink_drinker_levels/components/barrel_glow/barrel_glow.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
40 changes: 40 additions & 0 deletions .../lore_quests/quest_003/2_ink_drinker_levels/components/barrel_glow/barrel_glow.png.import
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
[remap]

importer="texture"
type="CompressedTexture2D"
uid="uid://brranfy51i56n"
path="res://.godot/imported/barrel_glow.png-05ec822d59b536030b7c5094e7b8b456.ctex"
metadata={
"vram_texture": false
}

[deps]

source_file="res://scenes/quests/lore_quests/quest_003/2_ink_drinker_levels/components/barrel_glow/barrel_glow.png"
dest_files=["res://.godot/imported/barrel_glow.png-05ec822d59b536030b7c5094e7b8b456.ctex"]

[params]

compress/mode=0
compress/high_quality=false
compress/lossy_quality=0.7
compress/uastc_level=0
compress/rdo_quality_loss=0.0
compress/hdr_compression=1
compress/normal_map=0
compress/channel_pack=0
mipmaps/generate=false
mipmaps/limit=-1
roughness/mode=0
roughness/src_normal=""
process/channel_remap/red=0
process/channel_remap/green=1
process/channel_remap/blue=2
process/channel_remap/alpha=3
process/fix_alpha_border=true
process/premult_alpha=false
process/normal_map_invert_y=false
process/hdr_as_srgb=false
process/hdr_clamp_exposure=false
process/size_limit=0
detect_3d/compress_to=1
46 changes: 46 additions & 0 deletions ...es/quests/lore_quests/quest_003/2_ink_drinker_levels/components/barrel_unlock_sequence.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# SPDX-FileCopyrightText: The Threadbare Authors
# SPDX-License-Identifier: MPL-2.0
class_name BarrelUnlockSequence
extends Node

@export var barrels: Array[FillingBarrel]
@export var auto_start: bool = true

var current_target_index: int = 0


func _ready() -> void:
if auto_start:
call_deferred("start_sequence")
Copy link
Member

@wjt wjt Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
call_deferred("start_sequence")
start_sequence.call_deferred()

Copy link
Member

@wjt wjt Feb 9, 2026

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?



func start_sequence() -> void:
randomize()

Comment on lines +18 to +19
Copy link
Member

@wjt wjt Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
randomize()

https://docs.godotengine.org/en/stable/tutorials/math/random_number_generation.html#the-randomize-method says:

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.

if barrels.is_empty():
push_warning("BarrelUnlockSequence: No barrels assigned.")
return

barrels.shuffle()
Copy link
Member

@wjt wjt Feb 9, 2026

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?

threadbare/scenes/game_logic/fill_game_logic.gd

Line 42 in 2942489

var filling_barrels: Array = get_tree().get_nodes_in_group("filling_barrels")

Some ideas:

  1. Remove the shuffle() call
  2. Keep it, but add an @export var randomize_barrel_order: bool variable (adjusting the default value to taste :) )
  3. If you think the mechanic is best enjoyed with a random order: keep the shuffle, but remove the @export from var barrels and instead populate that array in _ready in the same way that fill_game_logic.gd does


for barrel in barrels:
if not barrel.completed.is_connected(_on_barrel_completed):
barrel.completed.connect(_on_barrel_completed)

# Initial state: all locked
barrel.set_locked_state(true)

unlock_next_barrel()


func unlock_next_barrel() -> void:
if current_target_index < barrels.size():
var target: FillingBarrel = barrels[current_target_index]

if is_instance_valid(target):
Copy link
Member

@wjt wjt Feb 9, 2026

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?

target.set_locked_state(false)
Copy link
Member

@wjt wjt Feb 9, 2026

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:

Suggested change
target.set_locked_state(false)
target.is_locked = false



func _on_barrel_completed() -> void:
current_target_index += 1
unlock_next_barrel()
1 change: 1 addition & 0 deletions ...uests/lore_quests/quest_003/2_ink_drinker_levels/components/barrel_unlock_sequence.gd.uid
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
uid://en37d2lad0px
5 changes: 5 additions & 0 deletions scenes/quests/lore_quests/quest_003/2_ink_drinker_levels/ink_combat_round_6.tscn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
[ext_resource type="TileSet" uid="uid://b8qnr0owsbhhn" path="res://tiles/exterior_floors.tres" id="2_g1en3"]
[ext_resource type="TileSet" uid="uid://dfp36ffpanjq2" path="res://tiles/elevation.tres" id="2_s2t2y"]
[ext_resource type="Script" uid="uid://cp54mgi54nywo" path="res://scenes/game_logic/fill_game_logic.gd" id="3_1a5yo"]
[ext_resource type="Script" uid="uid://en37d2lad0px" path="res://scenes/quests/lore_quests/quest_003/2_ink_drinker_levels/components/barrel_unlock_sequence.gd" id="3_ihx08"]
[ext_resource type="TileSet" uid="uid://bjx3gvah0ycl1" path="res://tiles/foam_2.tres" id="5_4co4a"]
[ext_resource type="TileSet" uid="uid://do0ffypatd77h" path="res://tiles/bridges.tres" id="5_bl3l0"]
[ext_resource type="PackedScene" uid="uid://iu2q66clupc6" path="res://scenes/game_elements/characters/player/player.tscn" id="5_pbtyo"]
Expand Down Expand Up @@ -63,6 +64,10 @@ metadata/_custom_type_script = "uid://bgmwplmj3bfls"
[node name="BackgroundMusic" parent="." unique_id=1117240436 instance=ExtResource("1_7y6db")]
stream = ExtResource("2_1a5yo")

[node name="BarrelUnlockSequence" type="Node" parent="." node_paths=PackedStringArray("barrels")]
script = ExtResource("3_ihx08")
barrels = [NodePath("../OnTheGround/FillingBarrel"), NodePath("../OnTheGround/FillingBarrel2"), NodePath("../OnTheGround/FillingBarrel3")]

[node name="FillGameLogic" type="Node" parent="." unique_id=14736843]
script = ExtResource("3_1a5yo")
barrels_to_win = 3
Expand Down