search

SelinaDev / Godot-Roguelike-Tutorial

Yet Another Roguelike Tutorial in Godot
MIT License
78 stars 10 forks source link

bug with confusion scroll and radius of 0 #15

Closed ghost closed 7 months ago

ghost commented 7 months ago

when item targeting radius is 0, and you activate and cancel or drop the item. the game bugs out and stays in dummy input handler

test it by dropping a confusion scroll or activating it. so a targeting radius of 1 and above means its an AOE attack, but 0 means its a targeted single tile attack, and -1 no targeting (health potions activation etc) but the logic bugs out at a radius of 0.

I am not sure on how to fix it exactly. input handlers have been confusing so far.

SelinaDev commented 7 months ago

I can replicate the bug with dropping the item, but not with activating. I am not sure what you mean there. I do admit that input handling has turned out horribly, and it's not something I'm proud of in this project. Especially the dummy handler system is weird. By now I'm getting very annoyed with the handlers myself. Originally my problem was that when I chained a position selection after an item selection, the way I handled input handler switching meant that you would end up in the wrond state once you're in the position selection mode. That's why I hard-coded a check for -1 range into the get_item() function, so it would work together with the get_grid_position() function. However, I forgot that item dropping also needs to use that function, and in that case that check makes no sense. I fear that I will have to solve this with another hack, probably an extra function parameter for get_item(). I'll update it as soon as possible.

ghost commented 7 months ago

Alright, Thank you. I've been thinking about ripping out the input handler system entirely. and just have one node controlling all inputs. Shouldn't be too hard.

SelinaDev commented 7 months ago

I just committed a change to the code that should fix this issue. It does use an extra parameter for get_item(), but due to a default argument this only needs to be set in activate_item(), in order to tell get_item() that a targeting step afterwards is possible. Again, not the cleanest solution overall, but hopefully a relatively clean fix given the current state of the code.