Phazorknight / Cogito

Immersive Sim Template Project for GODOT 4
MIT License
717 stars 81 forks source link

Create new Wieldable #144

Closed ksjfhor closed 3 months ago

ksjfhor commented 3 months ago

Cogito and Godot Engine Version: Cogito: beta 202403.25 Godot: 4.2.1.stable

Description: I am too stupid to create a new wieldable, I tryed it on my own, until I found your YT-Video from yesterday in the docs, so I started fresh with the Video, from around 2 min until 5 min ( I watched the whole Video But there must be one thing that I miss, because the darts does not get marked as pickupable .. :/

Reproduction steps: Please see the screenshots what I did to the nodes. I basicly took a look on the laser rifle for the wieldable and the battery as ammo. I created and saved the InventorySlot for the dart and for the animations. Same for adding the Dart to the player and the PlayerInteractionComponent.

Expected behavior: I expect that I could pick up the darts and make them a wieldable to throw the dart.

Screenshots: grafik grafik grafik

grafik grafik

grafik grafik

grafik

BTW: Line 7 in the InventoryItemPD.gd is "despcription" not "description" ;) And I got confused by the CogitoClass-node, the node of the battery is a Rigidbody3D but the script extends a Node3D, is there a reason why ?

Phazorknight commented 3 months ago

Firstly, sorry that this is giving you trouble. I'm sure we can get this to work though.

As a start, what I find confusing is that with most wieldables, the wieldable item itself and it's ammunition are clearly separate items. In your case though, the wieldable dart arrow and the dart's themselves that work as ammo seem to use the same mesh. Additionally, since they'd be separate InventoryItems and require separate pickup objects, this might be quite confusing for the user as well.

Using the toy pistol, or the traditional separation between WieldableItem and AmmoItem might actually make this more complicated. I'd consider creating a custom Wieldable that is stackable as an item and gets unequipped and discarded on use, though that would get quite in the weeds with how Cogito's wieldable and inventory system works.

I'll try to create an item like that myself, since I think this could be useful for many other use cases as well.

Now that aside, your first issue seems to be with the dart as a pickup item. Can you double check that the collision layers are set correctly on the root node (the one with the Cogito_Object.gd script)? Collision Layer should be set to 1 and 2.

ksjfhor commented 3 months ago

No problem at all :) Thank you for the effort you put into this.

The pickup dart have set the collision on layer 1 and 2, mask only on 1.

BTW: Here are the files, maybe you can see something obvious. pickup_dartarrow.tscn.txt pickup_dartarrow.tres.txt The wieldable: dart_arrow.tscn.txt The script is a copy from the laserrifle, no logic so far changed, only set the export group to dart settings and put some values in.

Phazorknight commented 3 months ago

So I tried to have my own go at this, please check out https://github.com/Phazorknight/Cogito/commit/55d5c796596792409a479c9550ce5f420d1334c8

Check the Demo_Laboratory scene, you'll know find 4 pickup_darts there.

ksjfhor commented 3 months ago

This is so cool, but I wish I knew what I did wrong. I am going to create a new fresh wieldable, so maybe I can figure out my mistake(s).

I saw an Error about the Wieldable_Dart.gd:48 @ action_primary(): Animation not found: action_primary. So I added an animation "Dart_shoot", but it did not fix the error, let me left wondering were does "anim_action_primary" come from ? Animation is position of the dartmesh from 0s: 0.3, -0.2, -0.4 to 0.4s: 0.3, 0.2, -0.3.

As soon as I equip the dart i get: E 0:00:44:0142 WieldableItemPD.gd:69 @ update_wieldable_data(): Error calling from signal 'updated_wieldable_data' to callable: 'Control(Player_Hud_Manager.gd)::_on_update_wieldable_data': Cannot convert argument 3 from Object to Object. <C++ Source> core/object/object.cpp:1140 @ emit_signalp()

WieldableItemPD.gd:69 @ update_wieldable_data() WieldableItemPD.gd:52 @ take_out() WieldableItemPD.gd:41 @ use() InventoryPD.gd:64 @ use_slot_data() hot_bar_inventory.gd:21 @ _unhandled_input()
Phazorknight commented 3 months ago

I saw an Error

Yeah, I have them on my end too. there's still quite a few errors in there as it's quite hastily thrown together, but will try to clean this up this week and also probably rename the script to something like Wieldable_Throwable.

So I added an animation "Dart_shoot", but it did not fix the error

The animation name needs to match the string that's defined under the Animations tab under the CogitoWieldable itself. So in your case you need to change the anim action primary to Dart_shoot for the script to find it. The wieldable calls all animations based on these strings. image

As soon as I equip the dart i get...

Yeah i get the same. The way the HUD is updated isn't currently set up to support this kind of wieldable properly. Will look into fixing this.

I wish I knew what I did wrong.

The initial problem with not being able to pickup the pickups has nothing to do with the wieldable system. Will have to narrow it down, so I'd suggest trying to add other interaction components to your Cogito Object first and see if you can get these to work. Even if your item reference is broken, the Player Interaction component should still detect the object and display the interaction prompt.

Phazorknight commented 3 months ago

Following up, @ksjfhor please check out the latest https://github.com/Phazorknight/Cogito/commit/f84ba58fc7bca3d6a79f5f471b3f87c5b87bb618

I've cleaned up the way the wieldable_data is updated by checking if an WieldableItem needs/supports reload. The dart and the pickaxe both don't use reload, so their data gets displayed without ammo or charge.

I've also added a "throw" animation to the dart that the Wieldable references, so that error should now be gone as well.

ksjfhor commented 3 months ago

And again: Thank you :)

I feel like the throw animation should be not -0.5 on z, but rather -0.3 on z, but that can be personal preference.

One thing left, you may overread it: I am confused by the CogitoClass-node, the node of e.g. the battery is a Rigidbody3D but the script extends a Node3D. I know that it has to be a Rigidbody to apply physics, but why are you extending the "can-be-all-node" and not specifically a RigidBody ?

The Errors are all gone, and a 🥇 for #147

Phazorknight commented 3 months ago

I am confused by the CogitoClass-node, the node of e.g. the battery is a Rigidbody3D but the script extends a Node3D.

CogitoObject as a class doesn't necessary have to be a RigidBody3D as it's not using the RigidBody's functions/properties. The purpose of CogitoObject is to supply functions to enable persistency for save states as well as manage attached interaction components. There's times where a dev might want to make an AnimatableBody3D or something else a CogitoObject to take advantage of these, thus I extend it from the Node3D base class. Does that make sense?

ksjfhor commented 3 months ago

Yeah makes total sense and also a good idea. I would want to add or extend this to some dialog component, this way also the inventory from the fridge could be used for e.g. a trader npc. So from my side this issue can be considered as done, I will try to get other stuff done before I create a new wieldable ;)

Phazorknight commented 3 months ago

Sweet. I've also just renamed the Wieldable_Dart.gd to Wieldable_Throwable.gd to make it more clear that this script can be used for all kinds of throwable items (grenades for example)