Skip to content

fix(client): preserve vanilla modifier identifiers in synced updates#1997

Open
MarkPLacer wants to merge 3 commits intoPumpkin-MC:masterfrom
MarkPLacer:fix/attribute-modifier-duplication
Open

fix(client): preserve vanilla modifier identifiers in synced updates#1997
MarkPLacer wants to merge 3 commits intoPumpkin-MC:masterfrom
MarkPLacer:fix/attribute-modifier-duplication

Conversation

@MarkPLacer
Copy link
Copy Markdown

Summary

Follow-up to #1990.

This fixes duplicated client-side stacking for effect-driven attribute modifiers such as Speed by preserving vanilla modifier identifiers during attribute sync, and by properly marking attributes dirty when those modifiers are removed.

Root Cause

Pumpkin currently stores runtime attribute modifier ids as Uuids derived from the vanilla modifier id string, and then serializes those UUID-based identifiers back to the client.

Vanilla uses identifier strings for these modifiers, for example, minecraft:effect.speed. Because Pumpkin sends a UUID-form string instead, the Java client can treat the synced modifier as distinct from the vanilla effect modifier it already knows about and apply both.

There is also a cache invalidation problem on effect removal: remove_effect currently removes modifiers with a direct retain() on the vector, which bypasses AttributeInstance::remove_modifier() and never marks the attribute instance as dirty. That can leave stale effective values cached after the effect is gone.

Changes

  • Change Modifier.id from Uuid to String
  • Preserve vanilla modifier identifiers when applying and syncing effect-based attribute modifiers
  • Use remove_modifier() during effect removal so the attribute is marked dirty and recomputed
  • Update Enderman's custom movement speed modifier to use a stable string id

Impact

  • Prevents duplicated modifier stacking on the client for effects like Speed
  • Ensures cleared effects and post-death state sync back to the correct movement speed
  • Keeps custom non-vanilla modifiers working with explicit string identifiers

Known Limitations

  • This PR is intentionally scoped to modifier identifier sync and dirty-state handling
  • Any remaining effect behaviour differences outside attribute sync should be addressed separately
  • Reconnecting while an effect is active still appears to have a separate pre-existing sync issue: the client HUD effect is restored, but runtime attribute modifiers are not rebuilt from persisted active_effects on join

Testing

  • cargo check -p pumpkin
  • Verified against decompiled vanilla 26.1.1 and 26.1.2 sources for attribute modifier identifiers and attribute update packet behavior
  • In-game verification on Pumpkin:
    • Applying Speed no longer causes duplicated stacking
    • Reapplying the same effect does not increase speed further
    • clearing the effect restores normal movement
    • Dying with the effect active restores normal movement after respawn
    • Enderman's angry speed behaviour still works
  • Side-by-side behaviour check against vanilla 26.1.2

@Q2297045667 Q2297045667 added the bug Something isn't working label Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants