BUGJam/pounce-game
27
0
Fork 0
Code Issues Pull requests Projects 1 Releases 2 Packages Wiki Activity Actions

feat: allow placing any pickup item on pedestals #64

Merged
noahg merged 14 commits from noahg/feat-allow-placing-any-pickup-item-on-pedestals into dev 2026-04-09 07:53:46 +00:00
Conversation 13 Commits 14 Files changed 28 +331 -302
noahg commented 2026-04-08 07:42:37 +00:00
Member
Copy link
No description provided.
noahg self-assigned this 2026-04-08 07:42:37 +00:00
xury requested changes 2026-04-08 11:15:44 +00:00
Dismissed
xury left a comment
Copy link

Thank you for working on this, most of it seems to be working, but this introduces a crash when entering the drop point while carrying an item:

E 0:01:45:184   can_see_target: Invalid access to property or key 'global_position' on a base object of type 'null instance'.
  <GDScript Source>physics_utils.gd:18 @ can_see_target()
  <Stack Trace> physics_utils.gd:18 @ can_see_target()
                player_interaction_handler.gd:295 @ _update_current_interactable_object()
                player_interaction_handler.gd:344 @ _on_area_entered()

This seems to happen for any sell-able object, as well as objects that shouldn't be sell-able like the keycard.

Thank you for working on this, most of it seems to be working, but this introduces a crash when entering the drop point while carrying an item: ``` E 0:01:45:184 can_see_target: Invalid access to property or key 'global_position' on a base object of type 'null instance'. <GDScript Source>physics_utils.gd:18 @ can_see_target() <Stack Trace> physics_utils.gd:18 @ can_see_target() player_interaction_handler.gd:295 @ _update_current_interactable_object() player_interaction_handler.gd:344 @ _on_area_entered() ``` This seems to happen for any sell-able object, as well as objects that shouldn't be sell-able like the keycard.
👀 1
xury reviewed 2026-04-08 11:32:43 +00:00
xury left a comment
Copy link

I think it would be best to take the refactor one step further, and turn 'sculpture_puzzle_piece_slot.gd' into 'item_slot.gd'

This could be a base class that fires signals for place_item and remove_item instead of directly handling the logic, then the sculpture puzzle could be set up to subscribe to these signals in a more generic way.

This would also allow for designers to make new puzzles very easily, as well as use the item slot system for entirely new use-cases.

I think it would be best to take the refactor one step further, and turn 'sculpture_puzzle_piece_slot.gd' into 'item_slot.gd' This could be a base class that fires signals for `place_item` and `remove_item` instead of directly handling the logic, then the sculpture puzzle could be set up to subscribe to these signals in a more generic way. This would also allow for designers to make new puzzles very easily, as well as use the item slot system for entirely new use-cases.
entities/characters/player/player_interaction_handler.gd Outdated
@ -70,3 +82,3 @@
var sculpture_puzzle_piece_slot := focusable_object as SculpturePuzzlePieceSlot
if sculpture_puzzle_piece_slot and _held_item is SculpturePuzzlePiece:
if sculpture_puzzle_piece_slot and _held_item:
xury commented 2026-04-08 11:23:04 +00:00
Owner
Copy link

suggestion: As long as things are being made more generic and interchangeable with other systems in this PR, it would be could to eliminate specific wording around "sculpture puzzle piece", maybe call this "item_slot".

suggestion: As long as things are being made more generic and interchangeable with other systems in this PR, it would be could to eliminate specific wording around "sculpture puzzle piece", maybe call this "item_slot".
👍 1
noahg commented 2026-04-08 21:25:28 +00:00
Author
Member
Copy link

Good idea! Done.

Good idea! Done.
noahg marked this conversation as resolved
interactable_objects/puzzles/sculpture/sculpture_puzzle_piece_slot.gd Outdated
@ -10,3 +10,3 @@
update_configuration_warnings()
var _current_sculpture: SculpturePuzzlePiece
var _current_pickup_item: PickupItem
xury commented 2026-04-08 11:27:40 +00:00
Owner
Copy link

suggestion: even though the class this is referring to is called PickupItem the wording is getting a little confusing due to sentences like "Cannot remove pickup item from empty slot" - which is it? remove or pick up?

it might be simpler to just call this _current_item, and use wording like: "Cannot remove item from empty slot"

suggestion: even though the class this is referring to is called `PickupItem` the wording is getting a little confusing due to sentences like "Cannot remove pickup item from empty slot" - which is it? remove or pick up? it might be simpler to just call this `_current_item`, and use wording like: "Cannot remove item from empty slot"
👍 1
noahg commented 2026-04-08 21:25:51 +00:00
Author
Member
Copy link

I completely agree. Done.

I completely agree. Done.
noahg marked this conversation as resolved
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from f88410dee6 to 9c11a29f07 2026-04-08 17:49:42 +00:00 Compare
noahg commented 2026-04-08 17:53:34 +00:00
Author
Member
Copy link

@xury wrote in #64 (comment):

Thank you for working on this, most of it seems to be working, but this introduces a crash when entering the drop point while carrying an item:

E 0:01:45:184   can_see_target: Invalid access to property or key 'global_position' on a base object of type 'null instance'.
  <GDScript Source>physics_utils.gd:18 @ can_see_target()
  <Stack Trace> physics_utils.gd:18 @ can_see_target()
                player_interaction_handler.gd:295 @ _update_current_interactable_object()
                player_interaction_handler.gd:344 @ _on_area_entered()

This seems to happen for any sell-able object, as well as objects that shouldn't be sell-able like the keycard.

This should be resolved now. Looking into the other requests now.

@xury wrote in https://git.bugjam.dev/BUGJam/pounce-game/pulls/64#issuecomment-867: > Thank you for working on this, most of it seems to be working, but this introduces a crash when entering the drop point while carrying an item: > > ```text > E 0:01:45:184 can_see_target: Invalid access to property or key 'global_position' on a base object of type 'null instance'. > <GDScript Source>physics_utils.gd:18 @ can_see_target() > <Stack Trace> physics_utils.gd:18 @ can_see_target() > player_interaction_handler.gd:295 @ _update_current_interactable_object() > player_interaction_handler.gd:344 @ _on_area_entered() > ``` > > This seems to happen for any sell-able object, as well as objects that shouldn't be sell-able like the keycard. This should be resolved now. Looking into the other requests now.
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from 9c11a29f07 to db95bef602 2026-04-08 21:23:46 +00:00 Compare
noahg commented 2026-04-08 21:24:59 +00:00
Author
Member
Copy link

Addressed requested changes and rebased on dev.

Addressed requested changes and rebased on `dev`.
noahg requested review from xury 2026-04-08 21:25:05 +00:00
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from db95bef602 to 442ff0bd85 2026-04-08 21:40:45 +00:00 Compare
xury commented 2026-04-08 21:47:26 +00:00
Owner
Copy link

Thanks, this is looking really close now.
I did notice the console is being spammed with errors during regular gameplay:

E 0:05:49:875   physics_utils.gd:18 @ can_see_target(): Condition "!is_inside_tree()" is true. Returning: Transform3D()
  <C++ Source>  scene/3d/node_3d.cpp:642 @ get_global_transform()
  <Stack Trace> physics_utils.gd:18 @ can_see_target()
                player_interaction_handler.gd:300 @ _update_current_interactable_object()
                player_interaction_handler.gd:348 @ _on_area_entered()
                player_interaction_handler.gd:254 @ _put_down_item()
                player_interaction_handler.gd:186 @ _try_put_down_item()
                player_interaction_handler.gd:121 @ _input()

image

Thanks, this is looking really close now. I did notice the console is being spammed with errors during regular gameplay: ``` E 0:05:49:875 physics_utils.gd:18 @ can_see_target(): Condition "!is_inside_tree()" is true. Returning: Transform3D() <C++ Source> scene/3d/node_3d.cpp:642 @ get_global_transform() <Stack Trace> physics_utils.gd:18 @ can_see_target() player_interaction_handler.gd:300 @ _update_current_interactable_object() player_interaction_handler.gd:348 @ _on_area_entered() player_interaction_handler.gd:254 @ _put_down_item() player_interaction_handler.gd:186 @ _try_put_down_item() player_interaction_handler.gd:121 @ _input() ``` ![image](/attachments/97196905-a20b-4f60-8b74-415f88605eeb)
image.png
196 KiB
xury commented 2026-04-08 21:55:18 +00:00
Owner
Copy link

The sculpture_puzzle_piece scenes ended up with packed art asset mesh data inside of them, so they show up like this in the review:

image

Horrible, sorry I couldn't leave a comment in-place since the files weren't displayable in the review.

Let me push an update to these assets into dev to fix their art so that the data is referenced from the .gltf file instead of duplicated and packed into these scenes. Then we can rebase this PR on that.

The sculpture_puzzle_piece scenes ended up with packed art asset mesh data inside of them, so they show up like this in the review: ![image](/attachments/6b261f98-d238-4e3e-ace0-9c634d899ba2) Horrible, sorry I couldn't leave a comment in-place since the files weren't displayable in the review. Let me push an update to these assets into `dev` to fix their art so that the data is referenced from the .gltf file instead of duplicated and packed into these scenes. Then we can rebase this PR on that.
image.png
136 KiB
xury requested changes 2026-04-08 21:59:17 +00:00
Dismissed
interactable_objects/puzzles/item_slot/item_slot.gd Outdated
@ -0,0 +4,4 @@
signal solution_status_changed
@export var _solution_item: PickupItem:
xury commented 2026-04-08 21:59:11 +00:00
Owner
Copy link

I think this should be separated one more level, I think this is a good case for inheritance - ItemSlot will likely get used for lots of things that have nothing to do with puzzles, and therefore shouldn't have a "solution".

I think an inherited class is the right call here: PuzzleItemSlot - and it should be the thing to implement _solution_item.

Signals for when an item is placed / picked up from a slot are still valuable for the base ItemSlot class though.

I think this should be separated one more level, I think this is a good case for inheritance - ItemSlot will likely get used for lots of things that have nothing to do with puzzles, and therefore shouldn't have a "solution". I think an inherited class is the right call here: `PuzzleItemSlot` - and it should be the thing to implement _solution_item. Signals for when an item is placed / picked up from a slot are still valuable for the base ItemSlot class though.
noahg commented 2026-04-08 22:56:38 +00:00
Author
Member
Copy link

That's a good point. I'll split this into one more class.

That's a good point. I'll split this into one more class.
noahg commented 2026-04-08 23:33:39 +00:00
Author
Member
Copy link

Done! Now there is an ItemSlot class and a separate PuzzleItemSlot class.
I made the pedestal_item_slot.tscn scene inherit from item_slot_base.tscn, and in the statue room I override the script on the pedestal item slots with the PuzzleItemSlot class script.

Done! Now there is an `ItemSlot` class and a separate `PuzzleItemSlot` class. I made the `pedestal_item_slot.tscn` scene inherit from `item_slot_base.tscn`, and in the statue room I override the script on the pedestal item slots with the `PuzzleItemSlot` class script.
noahg marked this conversation as resolved
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from 442ff0bd85 to 7e8e67e1ca 2026-04-08 23:25:40 +00:00 Compare
noahg commented 2026-04-08 23:28:38 +00:00
Author
Member
Copy link

@xury wrote in #64 (comment):

Thanks, this is looking really close now. I did notice the console is being spammed with errors during regular gameplay:

E 0:05:49:875   physics_utils.gd:18 @ can_see_target(): Condition "!is_inside_tree()" is true. Returning: Transform3D()
  <C++ Source>  scene/3d/node_3d.cpp:642 @ get_global_transform()
  <Stack Trace> physics_utils.gd:18 @ can_see_target()
                player_interaction_handler.gd:300 @ _update_current_interactable_object()
                player_interaction_handler.gd:348 @ _on_area_entered()
                player_interaction_handler.gd:254 @ _put_down_item()
                player_interaction_handler.gd:186 @ _try_put_down_item()
                player_interaction_handler.gd:121 @ _input()

image

This should now be resolved. It was an order of operations issue where I was trying to set global_position before the node was part of the tree. I realized I only needed to set the local position. After making that change this error went away.

@xury wrote in https://git.bugjam.dev/BUGJam/pounce-game/pulls/64#issuecomment-887: > Thanks, this is looking really close now. I did notice the console is being spammed with errors during regular gameplay: > > ```text > E 0:05:49:875 physics_utils.gd:18 @ can_see_target(): Condition "!is_inside_tree()" is true. Returning: Transform3D() > <C++ Source> scene/3d/node_3d.cpp:642 @ get_global_transform() > <Stack Trace> physics_utils.gd:18 @ can_see_target() > player_interaction_handler.gd:300 @ _update_current_interactable_object() > player_interaction_handler.gd:348 @ _on_area_entered() > player_interaction_handler.gd:254 @ _put_down_item() > player_interaction_handler.gd:186 @ _try_put_down_item() > player_interaction_handler.gd:121 @ _input() > ``` > > [![image](/attachments/97196905-a20b-4f60-8b74-415f88605eeb)](/BUGJam/pounce-game/attachments/97196905-a20b-4f60-8b74-415f88605eeb) This should now be resolved. It was an order of operations issue where I was trying to set `global_position` before the node was part of the tree. I realized I only needed to set the local position. After making that change this error went away.
noahg commented 2026-04-08 23:30:56 +00:00
Author
Member
Copy link

@xury wrote in #64 (comment):

The sculpture_puzzle_piece scenes ended up with packed art asset mesh data inside of them, so they show up like this in the review:

image

Horrible, sorry I couldn't leave a comment in-place since the files weren't displayable in the review.

Let me push an update to these assets into dev to fix their art so that the data is referenced from the .gltf file instead of duplicated and packed into these scenes. Then we can rebase this PR on that.

I rebuilt the new nodes using their respective sculpture's pre-made scene and froze the RigidBody3D so they stay put. This should make it easier to rebase on your next changes.

@xury wrote in https://git.bugjam.dev/BUGJam/pounce-game/pulls/64#issuecomment-888: > The sculpture_puzzle_piece scenes ended up with packed art asset mesh data inside of them, so they show up like this in the review: > > [![image](/attachments/6b261f98-d238-4e3e-ace0-9c634d899ba2)](/BUGJam/pounce-game/attachments/6b261f98-d238-4e3e-ace0-9c634d899ba2) > > Horrible, sorry I couldn't leave a comment in-place since the files weren't displayable in the review. > > Let me push an update to these assets into `dev` to fix their art so that the data is referenced from the .gltf file instead of duplicated and packed into these scenes. Then we can rebase this PR on that. I rebuilt the new nodes using their respective sculpture's pre-made scene and froze the RigidBody3D so they stay put. This should make it easier to rebase on your next changes.
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from 7e8e67e1ca to 8f22fcfcce 2026-04-09 03:02:14 +00:00 Compare
noahg commented 2026-04-09 03:02:31 +00:00
Author
Member
Copy link

Rebased on dev.

Rebased on `dev`.
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from 8f22fcfcce to d32c726920 2026-04-09 03:36:41 +00:00 Compare
noahg commented 2026-04-09 03:38:45 +00:00
Author
Member
Copy link

Rebased on dev and incorporated the latest rebuilt statue scenes into the puzzle piece scenes.

Rebased on `dev` and incorporated the latest rebuilt statue scenes into the puzzle piece scenes.
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from d32c726920 to 390dfce5aa 2026-04-09 06:20:28 +00:00 Compare
noahg commented 2026-04-09 06:20:49 +00:00
Author
Member
Copy link

Rebased on dev.

Rebased on dev.
noahg requested review from xury 2026-04-09 07:06:12 +00:00
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from 390dfce5aa to 9b02ab1197 2026-04-09 07:14:37 +00:00 Compare
noahg force-pushed noahg/feat-allow-placing-any-pickup-item-on-pedestals from 9b02ab1197 to dcf8885ecb 2026-04-09 07:34:28 +00:00 Compare
xury approved these changes 2026-04-09 07:47:58 +00:00
xury left a comment
Copy link

Great work, this is a huge improvement for the sculpture room!

Great work, this is a huge improvement for the sculpture room!
🎉 1
noahg deleted branch noahg/feat-allow-placing-any-pickup-item-on-pedestals 2026-04-09 07:53:47 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
a_kimb0
TranquilMarmot
xury
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
noahg
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
BUGJam/pounce-game!64
No description provided.