peter-kish / gloot

A universal inventory system for the Godot game engine.
MIT License
660 stars 28 forks source link

Item Preview on drag is affected by the main node's position in the scene. #251

Open IrontMesdents opened 2 months ago

IrontMesdents commented 2 months ago

This issue was partially brought up by LeeWannacott about three weeks ago, but the new version doesn't seem to fix it.

A simple way of recreating the bug is to open the example scene inventory_grid_stacked_ex_transfer and change the position of the InventoryGridStackedTransfer Node to something else, as an exemple: x: 540 and y: 480. image

Then, you change the position of the first VBoxContainer to x:-540 and y: -480. image

If you run the scene, and drag any item, you'll end up with a preview that is offset by exactly -540 and -480 pixels.

This is an issue for every scene where the inventory is a child of another node, since the plug-in will use the top parent node as its reference. Any inventory system that pops a window dynamically inside a scene where the camera isn't exactly centered at 0,0 will have this offset. A solution would be to instead refer directly to the cursor position in the viewport for the item's position and not the scene's position. An other way would be to change the drag function and specify that it should use the CtrlInventoryGridEx as its reference point for the offset, which will make it adaptable wherever the window pops-up on screen.

other examples: Bug 1

Bug 2

I hope I'm being clear, I'm pretty new at game design and bug reports.

LeeWannacott commented 2 months ago

@IrontMesdents The last release was august 18th, so it wont have the latest changes. You can manually edit the file in [addons/gloot/ui/ctrl_inventory_grid_ex.gd] (in your project) to have the change and test if that fixes it for you: Here is the relevant commit: https://github.com/peter-kish/gloot/commit/7c69809030907703279fc21e6ffba4c60b000f91

IrontMesdents commented 2 months ago

I thought the issue was related to the _grab_offset variable in the ctrl_draggable.gd file. Modifying the at_position variable with a vector that would dynamically refer to the top node's origin would've probably fixed the issue, but again, I'm pretty new at this stuff so it might not be the actual issue. I wasn't sure about it so I decided not to mention it in my report.

I'll close if it does fix it, but it'll take a few days for me to get back home and try it out.

IrontMesdents commented 2 months ago

@LeeWannacott I edited the line you suggested in the commit, but as I feared, it didn't resolve my issue. The commit you mentionned adresses highlight issues while my issue is with the item preview being offset by the root node's position, not with the highlight. If I move my character in the scene, the item preview doesn't follow it, since the reference point for the preview isn't the ctrl_grid, but the main node of the scene.

I was partially able to show that the issue is in ctrl_dragable.gd by editing line 58 and adding a vector to the sub-preview's position: sub_preview.position = -Vector2(480,540) -get_grab_offset() It doesn't fix the issue dynamically, it just shows that the incorrect preview position seems to be related to that function. A patched-up fix would be to dynamically get the camera's position compared to the main scene's node and adjust it that way or find a way for it to track the mouse in the viewport directly.

IrontMesdents commented 2 months ago

I've found a partial fix by modifying the _get_drag_data function and adding this variable which modifies the subpreview's position. Here's the new function:

func _get_drag_data(at_position: Vector2):
    var sub_preview_pos = Vector2(get_viewport().get_camera_2d().get_screen_center_position().x-get_viewport_rect().size.x/2, get_viewport().get_camera_2d().get_screen_center_position().y-get_viewport_rect().size.y/2)
    if !_enabled:
        return null

    _grabbed_dragable = self
    _grab_offset = at_position  * get_global_transform().get_scale()
    dragable_grabbed.emit(_grabbed_dragable, _grab_offset)
    grabbed.emit(_grab_offset)

    var preview = Control.new()
    var sub_preview = create_preview()
    sub_preview.global_position = sub_preview_pos -get_grab_offset()
    preview.add_child(sub_preview)
    set_drag_preview(preview)
    return self

Now, every time the item is picked up, it's in the correct location, as long as you don't move the interface's position within the scene (which I want to do). This means the drag data would need to be updated like a _process function. It also relies on a Camera2D to work, but there is no guarantee that the interface will show up in a 2D environment so it's not an actual fix. It might however be a good lead to update it.

LeeWannacott commented 2 months ago

@IrontMesdents yes, sorry this is a separate issue from the highlighting, but the actual drag image being off centre, I think I reported this in an issue, I don't remember which one, I don't think it was fixed :thinking: ... I think I just bypassed the issue by putting inventory in its own canvaslayer.

IrontMesdents commented 2 months ago

@LeeWannacott No worries. The drag issue you mentionned was in the same post as the highlight issue. That's why that's what you suggested to me (and that's also why I posted the bug here separately too, since it was tagged as closed, but this issue wasn't resolved).

peter-kish commented 2 months ago

A simple way of recreating the bug is to open the example scene inventory_grid_stacked_ex_transfer and change the position of the InventoryGridStackedTransfer Node to something else, as an exemple: x: 540 and y: 480. Then, you change the position of the first VBoxContainer to x:-540 and y: -480.

Weird... I think I did exactly as you described: ss_drag_offset_01 ss_drag_offset_02 But I still don't get the described offset when dragging 🤔 drag_offset I even tried rolling back the changes to the last release (to v2.4.11), but I still can't reproduce the issue.

Edit: Btw @LeeWannacott, were you able to reproduce this?

LeeWannacott commented 2 months ago

Edit: Btw @LeeWannacott, were you able to reproduce this?

Yeah, I have this issue in my project , although the drag image is offset to the point its not visible on screen, so its less apparent (its only on shops) . I'll have a look at the example and see if I can reproduce it on that.

https://github.com/user-attachments/assets/25392c78-eb83-44ef-9e97-0be2c134200e

LeeWannacott commented 2 months ago

@peter-kish I can't reproduce it in the example inventory_grid_stacked_ex_transfer either (which is weird because I have this issue in my actual game, or maybe its separate because I don't think my nodes are positioned off centre :thinking: ). What version of Godot you running @IrontMesdents ? (In editor if you go help -> copy system info you can paste it) and also I guess what version of gloot?

IrontMesdents commented 2 months ago

@LeeWannacott @peter-kish My system info is this:

Godot v4.3.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated AMD Radeon RX 6650 XT (Advanced Micro Devices, Inc.; 31.0.24033.1003) - 11th Gen Intel(R) Core(TM) i7-11700K @ 3.60GHz (16 Threads)

Putting the Inventories on a Canvas Layer and changing line 58 of the ctrl_dragable.gd file to this seems to have resolved the issue on my side, even when my character is moving or the Canvas Layer is offset:

sub_preview.position = -get_canvas_layer_node().offset -get_grab_offset()

I'll just have to remember to always use a CanvasLayer when using inventories. Could the fact that you can't reproduce the bug be a result of a different project setting?

LeeWannacott commented 2 months ago

Godot v4.3.stable - Windows 10.0.19045 - Vulkan (Forward+)

I'm on Godot v4.3.stable - Pop!_OS 22.04 LTS - Wayland - Vulkan (Forward+). Unlikely to be a windows only bug :thinking:

Could the fact that you can't reproduce the bug be a result of a different project setting?

Possible, but I think its unlikely. I can't think of a setting that would effect it. Other than maybe a window setting :thinking:

I'll just have to remember to always use a CanvasLayer when using inventories.

I put stuff with my players UI on a CanvasLayer; so it doesn't spin around with the player.

IrontMesdents commented 2 months ago

Ok. Even more puzzling: I can't reproduce the bug on the example inventory on my side either. So that specific issue might be related to the highlighting issue somehow, but not the bug I mentionned?

Anyway. A way of fixing the issue in my case would be to recommend putting the UI on a specific CanvasLayer and editing the _get_drag_data function to this:

func _get_drag_data(at_position: Vector2):
    if !_enabled:
        return null

    _grabbed_dragable = self
    _grab_offset = at_position  * get_global_transform().get_scale()
    dragable_grabbed.emit(_grabbed_dragable, _grab_offset)
    grabbed.emit(_grab_offset)

    var preview = Control.new()
    var sub_preview = create_preview()
    if get_canvas_layer_node() != null:
        sub_preview.position = -get_canvas_layer_node().offset -get_grab_offset()
    else:
        sub_preview.position = -get_grab_offset()
    preview.add_child(sub_preview)
    set_drag_preview(preview)
    return self

I've added a single if/else statement in the ctrl_dragable.gd file to check if there is a canvas layer present and take its offset position to compensate if it's not positioned at 0,0. If there isn't a CanvasLayer, then the function runs the same as before. I'm satisfied with this solution, personally, but I'll leave it up to @peter-kish to decide if they want to investigate further. @LeeWannacott you might want to try it in your game, see if it fixes it for you too.

peter-kish commented 1 month ago

I did some experimenting with canvas layer offsets and this pretty much looks like a Godot issue. Seems like the drag preview set with Control.set_drag_preview doesn't take the offset into account at all. I was able to reproduce this with a simple scene and script. The canvas layer has an offset of (100,100), while the script only sets the drag preview: Screenshot_20241002_201806

The dragged control is then offset by exactly (100,100) pixels: Peek 2024-10-02 20-15

Update: https://github.com/godotengine/godot/issues/97748

MatteoGeusa-Dev commented 1 month ago

I think I solved your problem, make sure you don't set a zoom to your camera but enlarge the content instead, I attach video for you to see...

EDIT: However, there is a big problem, I can't scale the camera or resize the inventory, I'm stuck...

https://github.com/user-attachments/assets/61dd8b3d-1a9f-4ff1-a909-2ec34b9955b1