Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix swap of the main tile addon if a top object already exists in that tile #8936

Merged

Conversation

Districh-ru
Copy link
Collaborator

@Districh-ru Districh-ru commented Jul 7, 2024

The objects order on the same tile is determined by the current main addon object type (checked by getObjectTypeByIcn() function) and the layer type of the new tile being placed.

This PR fixes the getObjectTypeByIcn() function that was giving object type only for the main tile and giving OBJ_NONE for all other tiles. Now the function checks all object tiles.

It also forbids main addon swap if a new tile is the background layer one and the current tile already has an object with ID.

Master build:

fheroes2.2024-07-07.21-48-34-430.mp4

This PR:

fheroes2.2024-07-07.21-49-19-194.mp4

@Districh-ru Districh-ru added bug Something doesn't work editor Map editor related stuff labels Jul 7, 2024
@Districh-ru Districh-ru added this to the 1.1.1 milestone Jul 7, 2024
@Districh-ru Districh-ru self-assigned this Jul 7, 2024
Copy link
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 one comment here. Could you please take a look when you have time?

src/fheroes2/maps/maps_tiles_helper.cpp Outdated Show resolved Hide resolved
@Districh-ru Districh-ru requested a review from ihhub July 8, 2024 19:22
@ihhub ihhub requested a review from Branikolog July 9, 2024 13:12
@ihhub ihhub added the high priority Very critical change needed immediately label Jul 9, 2024
Copy link
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)

src/fheroes2/maps/map_format_helper.cpp Outdated Show resolved Hide resolved
src/fheroes2/maps/map_format_helper.cpp Outdated Show resolved Hide resolved
@Branikolog
Copy link
Collaborator

Hi!
If I place an artifact on a tile behind the castle - it still shown as artifact on right click on it:
image
After I pick it up the tile is shown as grass:
image
When the same cell without pre placed artifact belongs to castle area:
image

@Districh-ru
Copy link
Collaborator Author

Hi, @Branikolog, this bug also happens for the maps made by the original editor and we should fix it, but in a new PR IMHO. Because it is not Editor and new map format related but game engine related. :)

@ihhub
Copy link
Owner

ihhub commented Jul 10, 2024

Hi @Branikolog , this is the same issue as #8505 as it is not related to this PR.

Copy link
Collaborator

@Branikolog Branikolog left a comment

Choose a reason for hiding this comment

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

Okay. No more issues found then. :)

@ihhub ihhub merged commit e7d9ba0 into ihhub:master Jul 10, 2024
20 checks passed
@ihhub
Copy link
Owner

ihhub commented Jul 10, 2024

@Districh-ru , thank you so much for this fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something doesn't work editor Map editor related stuff high priority Very critical change needed immediately
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants