peter-kish / gloot

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

Performance: drag preview; running even while user not dragging. #192

Closed LeeWannacott closed 4 months ago

LeeWannacott commented 5 months ago

The code for drag preview is really hot, it runs even when not dragging.

This drag preview code has bad performance, which scales with how many inventories you have, even while not dragging. So, if you have a town with many merchants for me it takes ~2ms and I haven't even put all merchants in the town (have around 4). In actual game play where I am the only inventory it hovers around ~0.3ms.

I would suggest only setting global position and scale if the drag_preview is actually visible and also doing this in the physics_process, or alternatively on a timer with a sensible tick rate, perhaps between ~(24-60).

ctrl_draggable.gd:

func _process(_delta) -> void:
    if is_instance_valid(drag_preview):
        # the following lines run even when not dragging
        drag_preview.scale = get_global_transform().get_scale()
        drag_preview.global_position = _get_global_preview_position()
    if _show_queued:
        _show_queued = false
        show()

image

peter-kish commented 4 months ago

Yup, checking if it's visible before moving/scaling it should improve things. As for moving the logic out of _process: I'm a bit worried that the UI might feel a bit sluggish and not very responsive if it doesn't animate at max FPS 🤔

Anyways, I was thinking about simplifying the drag&drop code so that it uses Godot's builtin drag&drop system. That way the whole thing would be handled by Godot and there'd be no need for doing anything in _process.

...I think I'm starting to regret coming up with my own solution instead of using the available one because it's "not very customizable" ☹️

LeeWannacott commented 4 months ago

Yup, checking if it's visible before moving/scaling it should improve things. As for moving the logic out of _process: I'm a bit worried that the UI might feel a bit sluggish and not very responsive if it doesn't animate at max FPS 🤔

Anyways, I was thinking about simplifying the drag&drop code so that it uses Godot's builtin drag&drop system. That way the whole thing would be handled by Godot and there'd be no need for doing anything in _process.

...I think I'm starting to regret coming up with my own solution instead of using the available one because it's "not very customizable" ☹️

You can keep it in process if you want not a big deal, or maybe a timer that is set to the refresh rate cap of your monitor. I think the more important issue is it's running when you aren't even dragging , fix this and it's a just a bool check which shouldn't be that expensive anyway : 🤔

peter-kish commented 4 months ago

I agree. 4ff2eeef16aed2e3ab064b74faba9e33efca1dd9 will do for now and I'll refactor the whole thing at some later point.