Skip to content

Replace offset shifting animations with palette cycling#57

Open
morganchristiansson wants to merge 1 commit into
Return-To-The-Roots:masterfrom
morganchristiansson:palanim
Open

Replace offset shifting animations with palette cycling#57
morganchristiansson wants to merge 1 commit into
Return-To-The-Roots:masterfrom
morganchristiansson:palanim

Conversation

@morganchristiansson

@morganchristiansson morganchristiansson commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Using pos values from GameDataLoader broke the offset shifting animations that started crolling in unrelated textures.

Existing animations also jump as the offset resets in the animation cycle.

Do what s25client does - animate water and lava by cycling palettes.

Comment thread globals.cpp
// array for all palettes
std::vector<bobPAL> global::palArray(MAXBOBPAL);
// palette animation data per tileset (indexed by 8-bit tileset bmpArray index)
std::vector<std::vector<PaletteAnimData>> global::tilesetAnimData;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit hard to understand the comment as it only describes 1 vector, not which one nor the other one. maybe add a using PaletteAnimation = std::vector<PaletteAnimData> and name the variable vector<PaletteAnimation> tilesetAnimations or similar

Especially the above

    auto& animData = global::tilesetAnimData[tilesetIdx8];
...
    for(auto& anim : animData)

is confusingly named if anim is of type PaletteAnimData but animData is not.
You could also turn it around and renamed PaletteAnimData to PaletteAnimation and the vector of that is AnimData to match this code, but not sure if this is clearer.

Anyway such variable vs type naming mismatches are a clear indication of (mis)namings that need to be improved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants