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 757874 - Handle worm respawning on top of another worm
Handle worm respawning on top of another worm
Status: RESOLVED FIXED
Product: gnome-nibbles
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gnome-nibbles-maint
gnome-nibbles-maint
Depends on:
Blocks:
 
 
Reported: 2015-11-10 09:38 UTC by Iulian Radu
Modified: 2015-12-10 19:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for making worms transparent on respawn (5.24 KB, patch)
2015-11-20 19:33 UTC, Razvan Chitu
none Details | Review
Updated patch for making worms transparent on respawn (5.27 KB, patch)
2015-11-20 21:27 UTC, Razvan Chitu
none Details | Review
Patch for ghost bonus (6.24 KB, patch)
2015-11-21 19:31 UTC, Razvan Chitu
reviewed Details | Review
Updated patch for making worms transparent on respawn (5.31 KB, patch)
2015-11-30 13:50 UTC, Razvan Chitu
committed Details | Review

Description Iulian Radu 2015-11-10 09:38:39 UTC
> I think that the worms currently passing by the spawn point of the respawning
> worm should be able to do so without colliding with the respawning worm. We can 
> emphasize that but making the respawning worm transparent and have it stand 
> still until all tiles are clear.

This is the solution me and Michael agreed on in https://bugzilla.gnome.org/show_bug.cgi?id=754608

We're open to new or better ideas.
Comment 1 Razvan Chitu 2015-11-20 19:33:30 UTC
Created attachment 315988 [details] [review]
Patch for making worms transparent on respawn

I made it so worms are "dematerialized" upon respawn (they act as empty chars in the board, so any worm can move there). Also, they will only materialize if they are able to do so. If there is another worm on top of them, the materializing will be postponed.
Comment 2 Razvan Chitu 2015-11-20 21:27:56 UTC
Created attachment 315995 [details] [review]
Updated patch for making worms transparent on respawn
Comment 3 Razvan Chitu 2015-11-21 19:31:37 UTC
Created attachment 316027 [details] [review]
Patch for ghost bonus

On top of my previous patch, this one introduces a new bonus type that grants temporary transparency. The spawn settings and effect are for testing purposes and will probably be modified if it is decided that I can implement this.
Comment 4 Iulian Radu 2015-11-24 23:32:15 UTC
Review of attachment 315995 [details] [review]:

I'm not exactly sure if you should be able to move while dematerialized. The initial idea was to have the worm stand still until all tiles are clear, but maybe it's better the way you implemented it here.

What do you guys think?

::: src/worm.vala
@@ +276,3 @@
+    private void materialize (int [,] board)
+    {
+        foreach (var pos in list)

You should use brackets if you have more than one line inside the for loop.

@@ +293,3 @@
+        rounds_dematerialized = rounds;
+        is_materialized = false;
+        foreach (var pos in list)

Same as above.
Comment 5 Iulian Radu 2015-11-24 23:56:38 UTC
Review of attachment 316027 [details] [review]:

I'm not exactly sure if we want this bonus or not. My main concern is that is the animation is the same as the respawning one from the previous patch and it might cause some confusion while playing. I'd like to hear some opinions first.

::: src/nibbles-game.vala
@@ +404,3 @@
                         boni.add_bonus (board, x, y, BonusType.REVERSE, good, 150);
                     break;
+                default:

I don't think it's a good idea to add it on the default label. You should also tweak the number above (41), as it changes the pace of the game quite a bit.
Comment 6 Razvan Chitu 2015-11-30 13:50:25 UTC
Created attachment 316514 [details] [review]
Updated patch for making worms transparent on respawn

(In reply to Iulian Radu from comment #4)
> Review of attachment 315995 [details] [review] [review]:
> 
> I'm not exactly sure if you should be able to move while dematerialized. The
> initial idea was to have the worm stand still until all tiles are clear, but
> maybe it's better the way you implemented it here.
> 
> What do you guys think?
> 
> ::: src/worm.vala
> @@ +276,3 @@
> +    private void materialize (int [,] board)
> +    {
> +        foreach (var pos in list)
> 
> You should use brackets if you have more than one line inside the for loop.
> 
> @@ +293,3 @@
> +        rounds_dematerialized = rounds;
> +        is_materialized = false;
> +        foreach (var pos in list)
> 
> Same as above.

Fixed the brackets issue. The main reason for allowing worms to move on respawn is to not have them stuck in case an enemy worm feels like camping the respawn spot.
Comment 7 Razvan Chitu 2015-11-30 15:34:35 UTC
(In reply to Iulian Radu from comment #5)
> Review of attachment 316027 [details] [review] [review]:
> 
> I'm not exactly sure if we want this bonus or not. My main concern is that
> is the animation is the same as the respawning one from the previous patch
> and it might cause some confusion while playing. I'd like to hear some
> opinions first.
> 
> ::: src/nibbles-game.vala
> @@ +404,3 @@
>                          boni.add_bonus (board, x, y, BonusType.REVERSE,
> good, 150);
>                      break;
> +                default:
> 
> I don't think it's a good idea to add it on the default label. You should
> also tweak the number above (41), as it changes the pace of the game quite a
> bit.

Regarding the bonus, I was thinking about refactoring it and make it last until the first collision with another worm (or yourself). After the collision, the ex-ghost worm would now be flashing, indicating that the effect is going to expire. Also, maybe bonuses that are about to vanish from the board should also flash.

Regarding the value 41, I only used that for testing purposes as I wanted your opinion on the bonus itself.
Comment 8 Iulian Radu 2015-12-10 19:22:00 UTC
Review of attachment 316514 [details] [review]:

Looks good.
Comment 9 Iulian Radu 2015-12-10 19:26:08 UTC
I'm closing this as the bug has been fixed. You should submit a new bug report with your patch for the proposed bonus. That way, it will be easier to get some more opinions and make a decision.