Phazorknight / Cogito

Immersive Sim Template Project for GODOT 4
MIT License
670 stars 72 forks source link

Separates interactable updating from PlayerInteractionComponent to InteractionRayCast #173

Closed aronand closed 2 months ago

aronand commented 2 months ago

This PR holds multiple changes, the most notable being:

Closes #170

Depends on #169

(Due to there being lots of atomic commits, it may make sense to squash the commits depending on how you wish the repo history to look)

aronand commented 2 months ago

Regarding the drop prompt, you can actually observe it being broken in main as well at the moment. I suspect that could be fixed by actually adding state to the UI, possibly Player_Hud_Manager, which would then manage its own visibility reactively. EDIT: This is now fixed.

aronand commented 2 months ago

After creating a simplified version of the raycast for my own project I noticed that it and this are spamming the unseen signal. It's a really hard one to fix though, as is_instance_valid() will be true for both null and for a freed object (also null == freed object == true) => seemingly no easy way for an early exit.

Checking for Object.is_queued_for_deletion() also doesn't seem to help unfortunately. I'm possibly still missing something here, because in one of my attempts things seemed to work fine. In that one I had prints in different places which possibly slowed things down enough for everything to work as intended, and things broke when said prints were removed.

Thus this is currently an imperfect solution, as I haven't come up with a way to stop the signaling.

EDIT: Well, an hour more of testing and I've potentially found a solution in my own project:

# Handle freed objects.
# is_instance_valid() will be false for null and for freed objects, but only
# null will return 0 for typeof()
if not is_instance_valid(_interactable):
    if not typeof(_interactable) == 0:
        _interactable = null
        return

Still need to verify it works for Cogito as well.

aronand commented 2 months ago

A modified version of the simplified raycast is now in.

One oddity I still noticed is that at least in Cogito, the freed object handling sometimes gets triggered even when only quickly hovering the cursor over multiple interactables, but in practice this seems to have no significant impact. (Doesn't seem to happen in my own project, hard to say for sure, but possibly there's some sort of a bug elsewhere in Cogito that causes it) EDIT: This seems to have been caused by a null collider being an object null, which when assigned to be the _interactable will trigger freed object handling. Fixed (seemingly) by making collider a null type when: collider == null and typeof(collider) != 0

aronand commented 2 months ago

The drop prompt hack is now removed as well. This is achieved by not emitting interactable signals while an object is being carried.

With that I'd say the PR is feature complete. Let me know if you spot any bugs I may have missed.