Praytic / youtd2

Classic community-driven session-based Tower Defense game with RPG elements.
https://youtd2.com/
MIT License
97 stars 17 forks source link

Dangling reference in select_unit.gd #469

Closed PiotrKaszuba closed 1 month ago

PiotrKaszuba commented 1 month ago

During one of the games I've had following situation happen:

Kvel2D commented 1 month ago

I've managed to reproduce a similar, but not exactly the same, issue.

  1. Build a tower
  2. Spawn boss wave
  3. Select boss
  4. Deselect boss with Escape, keep hovering over boss
  5. Wait for tower to kill boss
  6. Now _units_under_mouse_list Array contains a dangling reference. Moving a mouse causes a breakpoint, also waiting for next creep to go under mouse causes a breakpoint.

2024-10-09 21_19_47-title_screen tscn - YouTD 2 - Godot Engine

The issue is with _on_unit_tree_exited() callback which is supposed to clear references when creeps die. It's used for both selection and hovering. Selection code in set_selected_unit() disconnects that callback, without considering whether it is still needed for hovering logic.

This issue is not exactly the same as the originally reported one, so there may be more deeper issues. For now, will fix the reproducible issue and add brute force check for valid references.

PiotrKaszuba commented 1 month ago

Thank you. The reproducible issue seems about the same without the first occurence of error that I've shown on the screenshot - the following occurences stopped at the exactly same point as the reproducible issue - when sorting, and with similar contents of _units_under_mouse_list. The reason for that would be no sorting actually took place the first time as that Array had only 1 element.

Kvel2D commented 1 month ago

I added a fix but need to double check the original issue. It might be different because my reproduced issue also had null SelectUnit._hovered_unit.

PiotrKaszuba commented 1 month ago

also had null SelectUnit._hovered_unit

I think the same was the case for me - I captured all local and member variables in the picture.

Kvel2D commented 1 month ago

In your case, you had _hovered_unit == "true null". In my case, _hovered_unit was a "previously freed reference", but displayed as "null" in editor.

I couldn't find any other issues which might cause dangling references in SelectUnit, so will close. The fix in dc36acf5c946e44b3ed5d5882729d424b85ab256 most likely solves both issues.

PiotrKaszuba commented 1 month ago

Thank you. I didn't know the difference between those null values nor I was able to spot it.