-
Notifications
You must be signed in to change notification settings - Fork 117
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
Separates interactable updating from PlayerInteractionComponent to InteractionRayCast #173
Conversation
Simplifies PlayerInteractionComponent by merging input checks for interact and interact2 to a single block. Removes RayCast3D.is_colliding() check and gets that information via RayCast3D.get_collider() (null if no collision)
Simplifies attempt_reload() by decreasing the level of nesting and by rewriting the reload logic: The reload now loops through the item slots, looking for suitable ammo. When ammo is found, the entire quantity is used instead of one by one.
InteractionRayCast emits a signal whenever an interactable enters or leaves the player's view.
…ponent InteractionRayCast already checks that collider is in the correct group. Thus this check is redundant in the PlayerInteractionComponent.
Not all interactables are CogitoObjects
Fixes UI prompt issues caused by implementation of InteractionRaycast. * freed objects are now handled by the raycast * player interaction component forces an UI update via signals after interaction
Gets rid of final UI prompt doubling issues by forcing an UI update each frame
attempt_reload() is seemingly supposed to be called if the player isn't looking at an interactable. This however doesn't seem to have worked as intended, so removing the check completely for now.
Adds a setter that handles emitting signals whenever the value of interactable changes. This ensures that the signals are emitted no matter what.
Moving all UI updates to a central location until a better solution for handling the UI is implemented
When the user picks up ammo for their currently wielded wieldable, the wieldables UI gets updated.
Adds a setter for carried_object which handles signaling started_carrying whenever a not-null value is assigned
Prompts are again only (re)built after the player interacts with something.
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 |
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 Checking for 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. |
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. |
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. |
A collider is or can be a null object instead of a null type. If it's a null object, change it to null so that _interactable isn't treated as a freed object.
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.
Very nice. Cleans up quite a few lines.
Thank you!
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)