guladam / deck_builder_tutorial

A roguelike deckbuilder tutorial project made in Godot 4.
MIT License
207 stars 30 forks source link

x_position gets offset in relic_handler visual by clicking fast #16

Closed Poromufflon closed 3 months ago

Poromufflon commented 4 months ago

-- extra information: i have everything scaled 2x (that's why the numbers in the log are diffrent) so i can use a 32x32 relic sprite, but i don't think this is the cause for this bug--

Issue: On 42:10 you say that you use the "if tween: tween.kill()" to kill the tween if there is already an active tween running.

I clicked fast with (TWEEN_SCROLL_DURATION := 0.2) but the problem was not the tweens colliding but rather the tweens stopping in the transition (my assumption) for some reason, creating the weird effect that x_position is a weird float (shifting the whole offset).

Screenshot 2024-07-06 185533

Fix / Avoid: To avoid this i disabled the left_button and right button at the start of the "tween_to" function and connected the finish Method to call update, so they basiclly enable proberly after the animation finished.

"tween.finished.connect( func(): update() )"

valigorevich commented 4 months ago

Confirming that the bug exists.

Bug occurs because when we press button during tween, we take current position of Relics, which can be any number because of tween animation.

Bug can easily be fixed by adding a new variable to store Relics position. By pressing the button we first increase or decrease relicls_position and then pass it to tween. Thus we are ensured that tween positions will always be valid. We also don't need to kill tween anymore. And player still can click buttons as fast as he wants to scroll relics faster.

var relics_position

func _ready() -> void: relics_position = relics.position.x

func _on_left_button_pressed() -> void: if current_page > 1: current_page -= 1 relics_position += page_width update() _tween_to(relics_position)

func _on_right_button_pressed() -> void: if current_page < max_page: current_page += 1 relics_position -= page_width update() _tween_to(relics_position)

guladam commented 3 months ago

I like this solution of introducing the new variable. I'll implement it this way, then close the issue. Thanks for contributing ☺️