drwhut / tabletop-club

An open-source platform for playing tabletop games in a physics-based 3D environment for Windows, macOS, and Linux! Made with the Godot Engine.
https://tabletopclub.net
MIT License
1.27k stars 55 forks source link

Improve the way object positions are changed when hovered. #140

Closed elmodor closed 1 year ago

elmodor commented 1 year ago

Describe the bug Picking up any object changes its position. When you pick up and object and drop it down again, it is moved downwards. To keep its current position you have to actually move the object up again. This is very tedious and can even lead to unwanted knock overs (in chess for example).

To Reproduce Steps to reproduce the behavior:

  1. Pick up any objects
  2. Drop it
  3. Notice that its location has changed

Expected behavior Picking up an object and dropping it without moving the mouse should not change its position.

Screenshots cards.webm chess.webm

Version v0.1.0 Beta 1

elmodor commented 1 year ago

Looking at the code this is currently intentional. Why is that so?

I would propose that instead changing the card to the mouse position, the card is only lifted in Y and then is glued to the mouse just like it was before. This is way more intuitive and less likely to screw over other objects (because the objects gets moved unintentionally into them).

Card being clicked to hover (in red the position of the mouse): on_ground

Now (old position transparent): hover_current

Proposal: hover_proposed

drwhut commented 1 year ago

I would say this is less of a bug, and more of a design flaw. And yes I do agree it can be improved.

Overall, in CameraController there are two things we can freely adjust when picking up objects:

Your proposal is essentially saying keep the y-position the same, but adjust the offset to match the piece's position before it was hovered.

One idea I had, that was very similar, was to change the hovering y-position to the y-position of the piece, and change the offset in the X and Z axes, which would be the same as your proposal, with the difference being the y-position is changed to match the piece's.

Maybe we could have a toggle in the options menu to decide whether the y-position is changed or not? If that is the case, which should be the default?

elmodor commented 1 year ago

I don't like the current system as default. When playing chess, whenever I pick up a piece, I bump into another piece because the object changes the position. I think this should not happen. Also when you pick a card up because you want to move it to the right or left, you have to move it up again because it was moved down.

Your proposal is essentially saying keep the y-position the same, but adjust the offset to match the piece's position before it was hovered.

I thought I wanted to change the y-position. I didn't quite get what you meant. Maybe we are talking about the same change.

I want to change the Y position only when starting hovering. Thus the object is lifted up - I don't want to use any information from the mouse position (except which piece was clicked to hover). If you take a look at my proposal image: the card on the bottom is the previous position of the card when it was being clicked by the mouse (red dot) - then it started hovering by moving up in Y. The result should be: If I don't move the mouse and let go again the piece should be at the same position as it was before:

hover.webm

remotesync func start_hovering(player_id: int, init_pos: Vector3, offset_pos: Vector3) -> void:
    if get_tree().get_rpc_sender_id() != 1:
        return

    hover_basis = transform.basis
    hover_offset = Vector3(0,8,0)
    hover_position = transform.origin + hover_offset
    hover_player = player_id

The offset with Y 8 is a bit much. Also just an example, doesn't work once the mouse moves (then the object jumps back to the mouse position)

drwhut commented 1 year ago

Sorry, I explained the y-position thing poorly.

The CameraController keeps an internal variable for the y-position when hovering pieces. This value currently does not change when hovering another object. The way the controller then calculates where to hover something is based on a line that is projected from the camera under the cursor, and where it intersects this y-position. This is also the variable that changes when using the scroll wheel when hovering objects.

When I said your proposal kept this variable the same, I meant that if you had hovered another object beforehand at a given distance from the table, then the next object you hover would go up to the same distance from the table as before (the same y-position).

Whereas in my proposal, the internal y-position would be set to the object's current y-position, so when you start hovering the piece, it doesn't actually move away from the table, only once you use the scroll wheel.

But in both examples, the offset is changed such that the object does not initially move in the X or Z axes (which solves the problem of knocking things over when picking objects up).

elmodor commented 1 year ago

does not initially move in the X or Z axes

Yeah that's right. That's basically the only thing I wanted to change.

only once you use the scroll wheel.

Hm, I don't think I would want that. I do like that it does hover a bit automatically. If you move cards/tokens around a lot you don't want to lift them up with the scroll wheel every single time. I think that would be a bit too much? I want to be able to pick up a card, move it and drop it. Changing the height every time would be very tedious. That's basically why I opened the other issue with containers resetting the current hover Y position: because I don't want to adjust this Y position every time I pick up an object.

The CameraController keeps an internal variable for the y-position when hovering pieces. This value currently does not change when hovering another object.

So basically I like how this is currently.

elmodor commented 1 year ago

It feels so much better now, thanks! :+1: