Simplifies interaction input handling and refactors reload logic to be more performant #169
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)
| if event.is_action_pressed("interact2"): | ||
| func _input(event: InputEvent) -> void: | ||
| if event.is_action_pressed("interact") or event.is_action_pressed("interact2"): | ||
| var action: String = "interact" if event.is_action_pressed("interact") else "interact2" |
There was a problem hiding this comment.
Input and InputEvent don't seem to have a way to return the action name, thus a bit hacky way of getting the action name. Needs a different solution if more interaction keys are introduced, or moving the interaction check to Node._process() instead, and/or checking action via:
if Input.is_action_pressed(node.input_map_action):
node.interact(self)This change alone seems to partially work, as the UI updates to reflect the new key, but the interaction itself doesn't seem to work with this change alone (EDIT: Probably because it's within a conditional checking for the two interaction keys 😄).
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.
| break | ||
| for slot in inventory.inventory_slots: | ||
| if slot != null and slot.inventory_item.name == equipped_wieldable_item.ammo_item_name and ammo_needed > 0: | ||
| inventory.remove_item_from_stack(slot) |
There was a problem hiding this comment.
InventoryPD.remove_item_from_stack() consumes one item at a time, making this loop take longer than is necessary. A possible refactor to it would be to allow the user to define the amount to remove as well.
There was a problem hiding this comment.
Overall this does some nice cleanups.
There's an issue though with the changes to the reload: It seems like the reload_amount properties of AmmoItemPD is not considered, thus a battery is just adding 1 to the charge of the flashlight. Or the clip of the Laser Rifle is just adding 1 to the Laser Rifle charge.
Would you be able to fix this?
|
Good catch! I'll have a look during the weekend. Mainly focused on testing the pistol and didn't realize the mechanics are different for different wieldables. |
|
Fix is in now. Tested both the flashlight and the laser rifle, which now seem to function as intended. |
aronand
changed the title
Reload time before:
Reload time after:
