Skip to content

Add the Maelstrom object#10309

Draft
Districh-ru wants to merge 42 commits intoihhub:masterfrom
Districh-ru:add-waterhole-object
Draft

Add the Maelstrom object#10309
Districh-ru wants to merge 42 commits intoihhub:masterfrom
Districh-ru:add-waterhole-object

Conversation

@Districh-ru
Copy link
Copy Markdown
Collaborator

@Districh-ru Districh-ru commented Oct 21, 2025

Many thanks to @PusshPop for the Maelstrom object sprites and for the underwater battlefield background!

fheroes2.2026-02-01.09-43-51-373.mp4

To properly represent the objects the water cycling colors were duplicated in the game palette to the non-cycle palette part. The same is done also for the lava cycling colors (it includes changes from #10287):

The new colors are placed in the unused part of palette (color IDs 246-253).
All engine palette conversion tables are updated to support the new colors (the values are mostly taken from their updated cycling colors in the corresponding table).

The Barrel object is also updated with the new palette colors.

TODO:

  • Add the logic for human player for the Maelstrom object;
  • Add the logic for AI player for the Maelstrom object.

@Districh-ru Districh-ru added this to the 1.1.12 milestone Oct 21, 2025
@Districh-ru Districh-ru self-assigned this Oct 21, 2025
@Districh-ru Districh-ru added editor Map editor related stuff new assets New game data related stuff, specific only to fheroes2 labels Oct 21, 2025
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

Comment thread src/engine/image_palette.cpp
Comment thread src/engine/image_palette.cpp
Comment thread src/engine/image_palette.cpp
Comment thread src/engine/image_palette.cpp
Comment thread src/engine/image_palette.cpp
Comment thread src/engine/image_palette.cpp
@ihhub ihhub modified the milestones: 1.1.12, 1.1.13 Nov 9, 2025
@ihhub ihhub modified the milestones: 1.1.13, 1.2.0 Dec 20, 2025
@ihhub ihhub added the high priority Very critical change needed immediately label Jan 30, 2026
@ihhub
Copy link
Copy Markdown
Owner

ihhub commented Jan 30, 2026

This is a must object for the upcoming release.

Hi @Districh-ru , do you think we can rework this pull request or would it be better to just from scratch? Also, we go updated images.

@ihhub ihhub modified the milestones: 1.2.0, 1.1.14 Jan 30, 2026
@Districh-ru
Copy link
Copy Markdown
Collaborator Author

Hi @ihhub
A little later (probably tomorrow) I'll try to resolve conflicts for this PR. If it'll be hard, I'll open a new PR.
But we should determine the logic for this object.
And also what updated images should we use?)

@ihhub
Copy link
Copy Markdown
Owner

ihhub commented Feb 1, 2026

Hi @ihhub A little later (probably tomorrow) I'll try to resolve conflicts for this PR. If it'll be hard, I'll open a new PR. But we should determine the logic for this object. And also what updated images should we use?)

Hi @Districh-ru ,

We should use images provided in this message on Discord - https://discord.com/channels/733093692860137523/1070399399127167106/1456756481612316693

I am attaching the archive here:
Waterhole close 2.zip

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

Comment thread src/fheroes2/maps/maps_tiles_helper.cpp Outdated
@Districh-ru
Copy link
Copy Markdown
Collaborator Author

Funny MSVC/clang-tidy errors:

  • clang-tidy says error: lambda capture 'waterholePartsX' is not required to be captured for this use [clang-diagnostic-unused-lambda-capture];
  • if I please Clang then MSVC says error C3493: "waterholePartsX" cannot be implicitly captured because no default capture mode has been specified
    So I used [&] to please them both.

@Mr-Bajs
Copy link
Copy Markdown
Contributor

Mr-Bajs commented Feb 21, 2026

How about fire spells under water?

@zenseii
Copy link
Copy Markdown
Collaborator

zenseii commented Feb 21, 2026

Lots of cool ideas here. I think any one you decide on will fit. Here are some more ideas you can take or leave:
Hp Regeneration for water elementals(we)
Increased speed for we

We'd also need to decide if and how these changes should apply to we in hero's army, both hired and summoned.

I also think we don't need to limit ourselves to reusing pre-existing spells and effects, in other words, we could make a new maelstrom-specific status for these we, and this status could, or could not, ensure the changes are exclusive to the neutral we.

As a side note in case anyone brings it up, these effects don't make sense for we in the moat since there's less water or some magical explanation 🤷

@Alucard648
Copy link
Copy Markdown

Go ahead, set time limit, i. e. 6 turns. If it expires, all living troops (undead and elementals not count) are insta-killed.
"ALL YOUR LIVING TROOPS HAVE DROWNED!" - the last entry in combat log, if it happens.

@ihhub
Copy link
Copy Markdown
Owner

ihhub commented Feb 22, 2026

Let's push these changes to the next release as it still needs a lot of refinement.

@ihhub
Copy link
Copy Markdown
Owner

ihhub commented Mar 10, 2026

Hi @Districh-ru , can we please do the following to progress with this pull request:

  1. Fix merge conflicts.
  2. Rename the object to maelstorm.
  3. Add compressed version of h2d file: everything except the header should be compressed. It is an additional step in computations but it makes the file much smaller. We don't care about backward compatibility here so it's fine to make changes. We will need to bump the version of the file.

@tlsa
Copy link
Copy Markdown

tlsa commented Mar 10, 2026

  1. Rename the object to maelstorm.

It should be maelstrom.

@Districh-ru
Copy link
Copy Markdown
Collaborator Author

Hi @Districh-ru , can we please do the following to progress with this pull request:

Hi @ihhub, done.

@ihhub ihhub changed the title Add the Waterhole object Add the Maelstrom object Mar 14, 2026
Copy link
Copy Markdown
Owner

@ihhub ihhub left a comment

Choose a reason for hiding this comment

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

Hi @Districh-ru , I left few more comments here. Can you please take a look at them?

Comment thread src/engine/h2d_file.cpp
}

std::copy_n( palette.begin(), paletteSize, PaletteHolder::instance().gamePalette.begin() );
auto & gamePalette = PaletteHolder::instance().gamePalette;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This function shouldn't have any logic except copying the whole palette. Here we are making some changes in the copied palette. We modified palettes in pal.cpp. Could you please explain why we need this change?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

pal.cpp and image.cpp have only changed palette mappings (what color to take instead of the given), but here we add the new colors (copies of cycling colors) to properly work in some engine functions functions.
Previously we did not add any colors to the palette but only remap them using "palettes" in pal.cpp and almost the same ones in image.cpp (in future we should reduce this duplication of palette mappings).

The GetPALColorId(), ApplyAlpha(), AlphaBlit() and some others relies on the game palette (vector of RGB values for each color), not on the mappings.
Without this change the fade-in/-out will be broken for the new object:
fheroes2 2022-12-19 19-45-29-933_

To fix it we will need to update the fheroes2::getGamePalette() function or update its result after every call that will introduce the colors adding operation on every call that is not good.

The problem is that we need to have the exact copy of 231, 232, 233 and 235 colors without cycling.

Comment thread src/fheroes2/heroes/heroes_action.cpp Outdated
Comment thread src/fheroes2/maps/maps_tiles_helper.cpp Outdated
Comment thread src/fheroes2/maps/maps_tiles_helper.cpp Outdated
@ihhub
Copy link
Copy Markdown
Owner

ihhub commented Mar 27, 2026

Hi @Districh-ru , I can see that this pull request changes a lot of things and I would like us to move forward with some changes. What we can do is to implement some changes in separate pull requests eventually reducing the length of the modified code here and ease the review of the changes.

Therefore, either me or @Districh-ru can create pull requests for the following changes:

  • enum class DelayType : uint8_t update
  • fix for fading animation for heroes
  • BattleInterface related optimizations

Then this pull request would be much easier to handle. What do you think @Districh-ru ? I can handle the creation of separate PRs if needed.

@Districh-ru
Copy link
Copy Markdown
Collaborator Author

Hi @ihhub, in the next days I'll open a PR with changes in battle_interface (including enum class DelayType : uint8_t) and a PR with hero fading animation fix. And probably a game palette update PR.

@ihhub
Copy link
Copy Markdown
Owner

ihhub commented Apr 3, 2026

Hi @Districh-ru , I am thinking that we are introducing a lot of changes in palettes and logic for it just because of the animation of the object. I think we should explore a possibility how we can achieve the same using the current palette without the need of introduction of new colors. I will have a conversion with @PusshPop in Discord about this.

ihhub added a commit that referenced this pull request Apr 3, 2026
All the changes have been taken from #10309
ihhub added a commit that referenced this pull request Apr 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editor Map editor related stuff high priority Very critical change needed immediately new assets New game data related stuff, specific only to fheroes2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants