die from falls / click

[marcomilone]

https://github.com/marcomilone/gnomelets

Given I LOVE your extension ❤️

Given I already put Guybrush sprite in the images folder as FIRST thing...

I implemented optional features:

• die by fall (half of screen height)
• die by click
• spawn offset

Spawn offset:
I use "dash-to-panel" extension: my panel is at the top of the screen, so windows start at y 32 px and Gnomelets are stuck at the top

check it out, if you like it I ask for pull request,

or take inspiration and do a job better than mine

Ideas:

• duck hunt mode
• parachute
• some Lemmings (videogame) ideas to steal?

let me know @ihpled !

enjoy

[ihpled]

Ciao Marco,
Great job on these new features! The physics calculations for the particles and the logic separating dragging from clicking add some fun new dynamics to the extension.

However, before we can merge this, there are a few issues (specifically regarding memory management and timers) that should be addressed. GNOME Shell extensions are very strict about cleanup, and unmanaged timers are one of the most common causes of crashes and rejections on the official extensions website (Ego).

Here is what should be fixed:

1. CRITICAL: memory leaks and unmanaged timeouts (GLib.timeout_add)

In _createSplatEffect, you are spawning numerous GLib.timeout_add timers for the particle animations, as well as timeouts for the 400ms delayed _respawn. None of these timers are currently tracked or cleared when the gnomelet is destroyed.

If a user disables the extension or locks their screen while a gnomelet is exploding, the destroy() method is called, which destroys the actors. However, since the timeouts are still running in the background, they will attempt to interact with the destroyed actors (particle.x += ... or this.actor.visible = true), causing Object St.Widget has been already disposed errors and potentially crashing GNOME Shell.

Solution: You must track all active particle actors and timeout IDs, and explicitly remove them inside the destroy() method.

Here's an example of how you can structure the cleanup:

// 1. In your constructor (or at the top of the file), initialize tracking arrays:
this._splatTimeouts = [];
this._particleActors = [];
this._respawnTimeoutId = null;

// 2. In _onClicked and _update, track the respawn timeout:
this._respawnTimeoutId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 400, () => {
    this._respawnTimeoutId = null; // Clear it when executed
    if (this.actor) {
        this.actor.visible = true;
        this._respawn();
    }
    return GLib.SOURCE_REMOVE;
});

// 3. In _createSplatEffect, track the particles and the burst/animation timers:
let particle = new St.Widget({ /* ... */ });
this._particleActors.push(particle);
parent.add_child(particle);

// ...
let timerId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 16, () => {
    // ...
    if (lifetime > safetyMax || particle.opacity <= 5) {
        particle.destroy();
        return GLib.SOURCE_REMOVE;
    }
    return GLib.SOURCE_CONTINUE;
});
this._splatTimeouts.push(timerId); // Track the active timer

And most importantly, update the destroy() method to clean everything up:

destroy() {
    // [ ... existing cleanup code (godModeTimeoutId, etc.) ... ]

    // Clean up respawn timeout
    if (this._respawnTimeoutId) {
        GLib.source_remove(this._respawnTimeoutId);
        this._respawnTimeoutId = null;
    }

    // Clean up splat animation and burst timeouts
    if (this._splatTimeouts) {
        this._splatTimeouts.forEach(id => GLib.source_remove(id));
        this._splatTimeouts = [];
    }

    // Destroy any lingering particle actors to avoid cluttering the UI Group
    if (this._particleActors) {
        this._particleActors.forEach(actor => {
            if (actor) actor.destroy();
        });
        this._particleActors = [];
    }

    // [ ... rest of the existing destroy method ... ]
}

2. Code style: nested if statements

In _update(), the logic evaluating if the gnomelet should splat relies on multiple stacked if statements without curly braces. This makes the code harder to read and prone to future indentation errors.

// Change this:
if (this._settings.get_boolean('death-by-high-falls'))
if (onGround) 
if (!this._godMode)
if (!this._isSplatted)
    {

// To this cleaner standard format:
if (this._settings.get_boolean('death-by-high-falls') && onGround && !this._godMode && !this._isSplatted) {

3. Stray debug statements (console.log)

Inside the fall checking logic, there are several leftover console.log statements checking fallHeight, limits, and coordinates. This will heavily spam the user's journalctl system logs. Please remove all debugging console.log statements before we merge.

---

4. Images directory structure

Please revert the change that moves the character assets from the images/ directory to an images/characters/ subdirectory (src/manager.js and src/prefs.js). Unless there is a specific reason for this restructuring, it's better to keep the original directory layout.

5. Duplicate README

Could you also remove the README.md file located inside the src/ directory? It appears to be a duplicate of the main README.md at the root of the project.

---

Once you update the PR with these life-cycle fixes and suggestions, I'll be happy to approve and merge it! Let me know if you need any help with the timeout tracking logic.

[marcomilone]

Hi ihpled...

In case you really like it, and you will accept my PR,
I'll do all you suggest.

What I've done is an humble exploration, to suggest you an eventual idea to implement.
Reading your response I understand your level is far over mine, and I thank you for the stimulation to do it by myself.

All your advice have so much sense.

What about the menus? Are ok?

In case you are really oriented to accept the PR, I will proceed (w my times...)

Thanks!

🙏

[ihpled]

Hi Marco,
yes, please, as I said, once you update the PR with these life-cycle fixes and suggestions, I'll be happy to approve and merge it.
Ofcourse, take all the time you need 😀!