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 Small Lighthouse Tower Floor #78628

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

WhaleYogurt
Copy link
Contributor

@WhaleYogurt WhaleYogurt commented Dec 17, 2024

Fix Small Lighthouse Tower Floor

Bugfixes: Lighthouse tower properly marked as inside space**


Summary

Bugfixes "Lighthouse tower properly marked as inside space"


Purpose of change

Fixes #78553.

Currently, the small lighthouse tower is marked as outside despite being enclosed with:

  • Stone masonry walls (t_stone_masonry_floor),
  • Glass walls at the ladder's top, and
  • A roof.

This causes unintended behavior, such as rain falling indoors. This PR ensures the lighthouse tower is correctly marked as an inside space.


Describe the solution

  • Added the t_stone_masonry_floor_balcony tile which is considered an inside floor.
    • This resolves the issue while preserving the aesthetic and structural integrity of the lighthouse.

Describe alternatives you've considered

  1. Manual tile replacement:
    Replace the t_stone_masonry_floor tiles specifically for the lighthouse with another floor tile (e.g., t_wood_floor or t_tile) to mark the space as inside. Please consider doing this @harakka so that sky island isn't affects

  1. Impact testing:
    • Checked for unintended side effects on other locations using t_stone_masonry_floor.

Notes

  • If modifying t_stone_masonry_floor globally is problematic, I will adjust the tiles specific to the lighthouse area.
  • Maintainers have permission to edit this PR as needed.

@WhaleYogurt
Copy link
Contributor Author

Just mostly reused the old PR text for times sack but in short I added a new tile which is indoors so that the old one doesn't conflict with sky island. So @harakka is now able to change the structure so it uses both

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Dec 17, 2024
@harakka
Copy link
Member

harakka commented Dec 17, 2024

This PR doesn't currently fix anything since the terrain you added isn't used anywhere. You can do it trivially yourself, add it to the palette in data/json/mapgen/lake_buildings/lighthouse_small.json and edit the lighthouse_small_z4 map further down in the same file to use it.

@WhaleYogurt
Copy link
Contributor Author

Thanks for telling me that. I know literally nothing about map-making in this game but I guess there's no time like the present. Any other resources I should know about for future reference?

@github-actions github-actions bot added the Map / Mapgen Overmap, Mapgen, Map extras, Map display label Dec 17, 2024
@WhaleYogurt
Copy link
Contributor Author

WhaleYogurt commented Dec 17, 2024

Does this work @harakka?

@Night-Pryanik
Copy link
Contributor

Yes, this will do.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inside of small lighthouse tower should be considered inside
3 participants