Skip to content

Add a "clicked" signal to Card3D.#21

Merged
tdecker91 merged 14 commits intotdecker91:mainfrom
camperotactico:add-card3d-clicked-signal
Oct 16, 2025
Merged

Add a "clicked" signal to Card3D.#21
tdecker91 merged 14 commits intotdecker91:mainfrom
camperotactico:add-card3d-clicked-signal

Conversation

@camperotactico
Copy link
Contributor

Hello again,

In a project I am working on using this asset I noticed it was difficult to read the text on certain cards. Because of that, I started working on a "pop up" window that displays all the details of the card. To show the window, the player has to tap on a card.

I noticed CardCollection3D already has a "card_clicked(card)" method, however I couldn't trigger a "tapping" or "clicking" action nicely without also triggering the "selection" on the DragController. This makes it so tapping on a card near the edge of a collection creates this card switching side-effect:

cards-switching-when-clicking-them.mp4

What this PR aims to do is solve this issue without modifying too much of the existing code:

  • Firstly, Card3D now includes a "click_threshold" timer and a "card3d_clicked" signal:
    1. Once the "mouse button pressed" event is received, the timer is started. If the "mouse button released" event is received during the threshold duration, the card only emits the new signal.
    2. If the threshold timer times out, the existing "card3d_mouse_down" signal is emitted and when the "mouse button released" event is received the "card3d_mouse_up" is fired.
  • Secondly, "CardCollection3D" has been changed to include a "card_delesected(card)" signal that gets fired when "card.card3d_mouse_up" is received. The "card_clicked(card)" signal is now sent when "card.card3d_clicked" is received.
  • Finally, I have updated the examples to work with the new changes . This means this changes will break compatibility with the current version

As it can be seen in the video below, the cards can now be clicked without causing the switching effect, and they can also be dragged as usual.

cards-not-swithcing-after-the-change.mp4

Let me know what do you think about this change and if you have any suggestion.

@tdecker91
Copy link
Owner

tdecker91 commented Oct 2, 2025

Welcome back @camperotactico

Thanks again for your great insight. I have some feedback on the change you've suggested. I understand the issue you're running into and recognize the strange single click behavior. I don't know if the timer implementation works well with the current state of things in the library.

  1. It makes the motion feel a little clunky and can lead to strange card animations. Or delays if moving cards around quickly feels like it is lagging or is not responsive. (even if the click threshold is small) I take inspiration from hearthstone and value having very quick and fluid motions
  2. Card3d nodes often become stuck in the "hovered" state. As demonstrated by the video below.
Recording.2025-10-01.234304.mp4

You're on the right track. I think the problem is in the interaction between card_collection_3d.gd and drag_controller.gd. The card collection 3d is what emits the signals the drag controller interprets to know when to begin a drag, etc...

It currently behaves like this:

  1. [card_collection] on card 3d mouse down emit card_selected
  2. [drag_controller] on card_selected start dragging
  3. [drag_controller] on input event when _dragging and mouse is released stop dragging
  4. [card_collection] on card 3d mouse up, emit card_clicked

This causes the behavior you showed in the video because it starts a drag event, and recognizes your mouse position is in a place where it should swap cards.

I think a proper solution could look something like one of these approaches

  1. don't start drag in the drag_controller until we've receive a mouse motion after selecting a card
  2. change the order of events from the drag_controller. So maybe don't emit a "card_selected" event right away. Emit card_selected:
    • after mousedown, and then subsequent mouse move
    • OR (maybe) after just clicking the card. Like hearthstone. A single click will grab the card for you to drag around. And this can be configurable

That could give a better control of receiving click events and when to start dragging or to ignore it for a click. And we wouldn't need to rely on a timer to delay action.

let me know what you think.

@camperotactico
Copy link
Contributor Author

Hello Tyson, thanks for the elaborate response. I hadn't realised of the issues my changes brought to the project. It is a good thing you gave the branch a try. I am going to revert the changes, take a different a different approach following your suggestion and update the PR as soon as I can.

I will get back at you if I find any issue with the new approach.

@camperotactico
Copy link
Contributor Author

Hey, I have done some thinking and I
I agree that checking for mouse motion events to handle clicks instead of a timer is a great improvement. But before I start implementing it into the code, I wanted to share with you some thoughts to make sure we are on the same page and I can keep the existing code as untouched as possible.

At the moment, 'card_collection_3d.gd` emits a "card_selected()" signal when a card is just pressed and then "card_clicked()" when the card is released. I believe the naming for this last signal is problematic and should be refactored to "card_deselected()" as it is closer to what it really does:

Every time a card is dragged around within its collection (to rearrange the cards, for example) the signal is emitted while I would expect that signal to only be emitted when the card is pressed and released without dragging it around.

The card collection really isn't handling any click logic, and I am not sure the drag controller should either, as it would make it have two responsibilities.

Perhaps we could just refactor that signal and then come up with a new class like "CardClickHandler" that could, optionally, be added to the scene and would check for both CardCollection and DragController signals and determine when cards are clicked.

What do you think?

@tdecker91
Copy link
Owner

tdecker91 commented Oct 3, 2025

Yes, I think what you described about the events selecting and deselecting is appropriate. That would make the card collection a bit more cohesive. It doesn't make sense to emit a card_clicked anytime releasing the mouse button from the collection.

I like the idea of CardClickHandler as well. But I think handling that within CardCollection3D could make the most sense. Because the card collection would need to keep track of weather or not to "select" a card. It would then inherently know to emit a card clicked if the mouse wasn't moved before the mouse up was received. (and then based on configuration it could also support selecting a card with a click)

However, CardCollection is not listening for mouse move events currently. I haven't thought about exactly what it might look like yet.

But, I'm open to either idea. 😄

@camperotactico
Copy link
Contributor Author

Great, I will update the signal name for the time being.

As for CardCollection3D handling the clicks, I can picture the implementation you suggested and I feel it will work fine. I would have to spend some time coming up with a nice and clean way to toggle "selecting on click" from the inspector.

I noticed every element on this asset use mouse inputs. I understand this is so that Card3D, CardCollection3D or DragController can work even when used separately. Do you think it would be a good idea to move all input handling to a separate script (or maybe even a Resource) so that all these classes can tap into it?

@tdecker91
Copy link
Owner

tdecker91 commented Oct 6, 2025

for what it's worth I don't think we'd necessarily need to implement the "selecting on click" option now. I was thinking it could be a possibility in the future.

About adding input to a separate script or resource, I haven't thought much about what that could look like. If you have a concrete idea of what you're thinking of I'd be happy to hear. I'm still a Godot novice ha.

But generally I am happy with the current state of event handling as I think they all have separate concerns.

  • Card3D listens to mouse_over and mouse_down signals from it's own StaticBody3D, then conveys those upwards
  • CardCollection3D listens to mouse entering / leaving it's own dropzone and emits signals for that
  • DragController is the only script I think actually reading any mouse event via func _input(event): It does this to update position of dragged card as you move it around.

This more or less matches the pattern we'd see in Godot where StaticBody3D for example handles the mouse events and the parent script connects to signals on the StaticBody3D. Then each component can behave well in isolation, but then the parents can decide superseding logic, like in CardCollection3D for example can disable hover state for other cards after you have selected a card to drag.

@camperotactico
Copy link
Contributor Author

Forget what I suggested about the inputs, I didn't had the code in front of me and I was writing from memory so I got confused about what parts of the code where doing what.

I have just pushed some commits that:
1- revert my previous changes
2- refactors card_clicked to card_deselected and its occurrences in the examples.
3- Changes DragController to only start a drag if the mouse is moved at least 10px (to avoid accidentally dragging if a mouse is too sensitive)

I still haven't add the ClickHandler component. I am going to wait until you check the changes I just submitted, just to make sure I am on the right track.

On a different note, I realised the DragController listens to InputMouseButton events to stop the drag, but since the CardCollection3D has a card_deselected signal, I thought it would be nicer if it used this signal instead to stop the drag.

However, when I tried that, I noticed Card3D was not sending a mouse_up signal the moment it was moved around the screen. After some research, I learnt that enabling Capture On Drag in the StaticBody3D of Card3D solved this issue.

This resulted in a more clean solution, where the mouse_up and mouse_down events were being sent from the Card -> Collection -> DragController. I reverted this change because then I found a weird bug which I couldn't properly trace: When I dragged a card outside the game window, only from the bottom of it, the card wouldn't send the mouse_up signal anymore.

I spent a bit of time trying to figure out what was causing it, but after a while I gave up. I wrote all of this in case you want to replicate the issue and hopefully can figure out what I was missing.

I will work on the ClickHandler as soon as I have some more time.

@tdecker91
Copy link
Owner

tdecker91 commented Oct 8, 2025

This is looking pretty good. The behavior is very clean now.

I recall running into the same issue of missing signals when moving the mouse off screen while dragging. And to be honest I assumed it was a godot bug, because I saw the behavior was different an OSX and Windows. Or maybe it's entirely platform dependant? 🤷‍♂️ So I settled on listening for the mouse up event like it is currently.

I do want to look at the changes a bit more thoroughly because I have noticed a few times where card collections are stuck in the "drop preview" mode. It doesn't happen often and maybe a preexisting problem, but I have only just noticed it with this change.

I did take the liberty of upgrading to godot 4.5 on the main branch to reduce some of the clutter on this MR. Do you mind rebasing so this MR is can be strictly just the fix of the "clicked" signals.

apart from the godot 4.5 upgrade changes the "imported_formats" were changed on many of the imported textures. like

path.etc2="res://.godot/imported/card_clubs_05.png-d38a985d9ec65a23bd89b3db5c41324f.etc2.ctex"
"imported_formats": ["etc2_astc"],

I'm not familiar with the difference. Was this an intentional commit? I tried looking up the difference and am still not sure which option is better.

@camperotactico
Copy link
Contributor Author

Hello, I just rebased this branch with your latest version of main. I see now that the changes in the texture imports here are not the same as in main. I haven't modified the import settings on purpose, it happens automatically every time I open the project. I am working on a Linux ARM64 machine, so I wonder if that is triggering the editor to reimport the textures? I am going to look for more information online to see if someone else is having the same issue. For the time being, I can revert those changes if you rather not modify them.

I will look into the dragging out of the screen bug. If I am able to replicate the issue on an empty project I will report a bug to the Godot engine.

@tdecker91
Copy link
Owner

tdecker91 commented Oct 8, 2025

ok cool.
yeah for now lets leave out the other texture changes and keep this isolated to just the functional changes.

If there's a good reason to update the texture stuff that can be a separate pull request

@camperotactico
Copy link
Contributor Author

Hello again, I just pushed a new commit that addresses the feedback you left + removes the texture import changes. If everything is right, please do not merge yet as I still have to work on the click feature.

@camperotactico
Copy link
Contributor Author

I recall running into the same issue of missing signals when moving the mouse off screen while dragging. And to be honest I assumed it was a godot bug, because I saw the behavior was different an OSX and Windows. Or maybe it's entirely platform dependant? 🤷‍♂️ So I settled on listening for the mouse up event like it is currently.

I forgot to mention this, but I created an issue in the Godot engine for this particular problem: 111410, and today it got marked as a bug.

@camperotactico
Copy link
Contributor Author

Hello again @tdecker91

I just pushed a commit that includes a card_clicked(card) signal into the CardCollection3D as you proposed. I decided not go for the discussed CardClickerHandler component as the logic needed for the click was not much and I felt that moving it to a separate script would have just obfuscated things. I have also updated the example test scene so it supports clicking for the hand and table cards.

In short, what now happens is that the CardCollection3D might emit card_clicked(card) on top of card_deselect(card). This signal is only emitted if the new variable is_dragging_card is false. This variable is set to false every time card_mouse_down is received, and is set to true from DragController when a drag stops.

Ideally, the variable should be set to true when a drag starts and false when a drag stops, but this won't work at the moment because of the workaround to the off-screen dragging bug: Currently, DragController stops a drag (and moves cards from one collection to another) BEFORE the dragged card emits card_mouse_up, which means the collection checking whether it should send a card_clicked(card) signal is not always _drag_from_collection.

@tdecker91
Copy link
Owner

Looks great, thanks for doing this 👍

@tdecker91 tdecker91 self-requested a review October 16, 2025 20:40
@tdecker91 tdecker91 merged commit 8034569 into tdecker91:main Oct 16, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants