After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 733429 - Check worm length after each turn
Check worm length after each turn
Status: RESOLVED FIXED
Product: gnome-nibbles
Classification: Applications
Component: general
git master
Other All
: Normal normal
: ---
Assigned To: gnome-nibbles-maint
gnome-nibbles-maint
Depends on:
Blocks:
 
 
Reported: 2014-07-19 23:26 UTC by Michael Catanzaro
Modified: 2014-08-19 00:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Regularly assert each worm is the proper length (2.07 KB, patch)
2014-07-19 23:26 UTC, Michael Catanzaro
none Details | Review
Fix worm length after respawn (893 bytes, patch)
2014-07-19 23:26 UTC, Michael Catanzaro
none Details | Review
Add a couple more good assertions (1.00 KB, patch)
2014-07-20 17:48 UTC, Michael Catanzaro
none Details | Review
Fix corner-case handling of tail teleportation (4.01 KB, patch)
2014-07-20 22:11 UTC, Michael Catanzaro
none Details | Review

Description Michael Catanzaro 2014-07-19 23:26:04 UTC
Better to crash immediately and trigger distro bug reporting tools than force the player to die on invisible worms. Best if this helps us figure out what's going wrong.
Comment 1 Michael Catanzaro 2014-07-19 23:26:06 UTC
Created attachment 281210 [details] [review]
Regularly assert each worm is the proper length

No bug can withstand the awesome power of invariants! Yeah!
Comment 2 Michael Catanzaro 2014-07-19 23:26:09 UTC
Created attachment 281211 [details] [review]
Fix worm length after respawn

We accidentally leave a one-space invisible position of death behind our
worm at all times after the worm respawns (for the duration of the
current map). Invisible positions of death are bad so let's get rid of
it.
Comment 3 Michael Catanzaro 2014-07-20 16:30:12 UTC
(In reply to comment #2)
> We accidentally leave a one-space invisible position of death behind our
> worm at all times after the worm respawns

...even with this patch, this still happens sometimes. :/
Comment 4 Michael Catanzaro 2014-07-20 17:48:55 UTC
Created attachment 281258 [details] [review]
Add a couple more good assertions

After we finish moving, the square our tail is on ought to be marked as
such. This assertion likes to fail when we trigger the teleporter bug.
Comment 5 Michael Catanzaro 2014-07-20 19:39:34 UTC
(In reply to comment #4)
> After we finish moving, the square our tail is on ought to be marked as
> such. This assertion likes to fail when we trigger the teleporter bug.

FYI: this should not be pushed (at least not yet) since it triggers EVERY time we go through a teleporter
Comment 6 Michael Catanzaro 2014-07-20 22:09:44 UTC
(In reply to comment #5)
> FYI: this should not be pushed (at least not yet) since it triggers EVERY time
> we go through a teleporter

Safe now, after applying all patches in bug #654072
Comment 7 Michael Catanzaro 2014-07-20 22:11:46 UTC
Created attachment 281270 [details] [review]
Fix corner-case handling of tail teleportation

What we do here is really insane. After our tail moves into the
teleporter, we erase it from the text board and remove its clutter
actor, but then intentionally leave the tail pointer stationary,
trusting to the gnibbles_worm_get_tail_direction function to position it
properly on the next turn, then warping and moving the tail pointer an
extra time on that turn. This kind of works because
gnibbles_worm_get_tail_direction() operates on the clutter actors, which
are stored in a linked list and do know their position. But it's also
super fragile if we do anything interesting with the teleporters (e.g.
teleport immediately on the next move, because we came at a teleporter
from a particular direction). So instead of this
don't-move-then-move-twice nonsense, assume that if we ever do not know
which direction to move in it's because we're at a teleporter, and try
to use it.

Also, add a couple sensible assertions to make sure this assumption
always holds.

Very pleased with this patch; I think there is still a problem if we teleport onto ourselves or another worm, which I still need to fix, but I THINK everything else is resolved. (A little worried there may still be a problem when respawning, but not sure because it's rare.)
Comment 8 Michael Catanzaro 2014-07-20 22:13:45 UTC
Comment on attachment 281270 [details] [review]
Fix corner-case handling of tail teleportation

Wrong bug.
Comment 9 Michael Catanzaro 2014-07-20 23:27:05 UTC
Two remaining problems here:

* You can respawn on top of another worm
* Respawns randomly and rarely fail horribly, not sure why