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

Structures can generate in saltwater and below mantle #3564

Open
AreYSerious opened this issue Feb 7, 2024 · 12 comments
Open

Structures can generate in saltwater and below mantle #3564

AreYSerious opened this issue Feb 7, 2024 · 12 comments
Labels
department: code Issues apparently caused by code priority: medium Bugs and nuisances that need attention status: new This issue is fresh!

Comments

@AreYSerious
Copy link
Contributor

Game Version

v1.19.3

Platform

Windows

Modded

Modded

SP/MP

Multiplayer

Description

The code for structure is not checking for saltwater. Also the underwater checks are insufficient. it's only checking the corners to my information. I'm guessing performance reasons.

image-12
image-14
image-28

How to reproduce

You can use my mod betterruins or vanilla and fly around with oceans enabled.

Screenshots

2024-02-02_23-48-56
image-25
image-13

Logs

Log

@AreYSerious AreYSerious added the status: new This issue is fresh! label Feb 7, 2024
@Craluminum2413 Craluminum2413 added priority: medium Bugs and nuisances that need attention department: code Issues apparently caused by code labels Feb 7, 2024
@AreYSerious
Copy link
Contributor Author

2024-02-08_19-46-19

@Barhandar
Copy link

Underwater generation itself fits perfectly with "the world got scrambled" idea implied by the lore. The issue is generating things that should not be there - grass, regular water, etc.

@AreYSerious
Copy link
Contributor Author

@Barhandar
This is more from a Modder point of view.

And the system atm should prevent complete underwater spawns so this is a bug.
Ruins in shallow water is intended but not like this.

@Barhandar
Copy link

IIRC the problem with "shallow" vs "not shallow" is that the game doesn't actually deal with saltwater correctly at all, as evidenced by [this comment](https://mods.vintagestory.at/show/mod/8969#cmt-19114 (the code checks for nonexistent "seawater" block rather than existent "saltwater"), besides lacking a check for water depth.

@AreYSerious
Copy link
Contributor Author

Yeah with the the problem with the saltwater is why this issue has been posted.

The shallow water is bcs structures only get checked on each corner for performance reasons so the there is no water it will spawn that's why they can spawn in shallow water

@radfast
Copy link

radfast commented Feb 23, 2024

I don't think the issue is specific to salt water, the code checks for any kind of water at the 4 corners. Maybe seen more on high ocean worlds just because there is more, and more deep, water on such worlds.

The issue here is the checking code is buggy, it is checking at heights based on the centre position of the ruin, but in the 4 corners being checked, those checks may be inside rock, if the lake / seabed is uneven. No water of any kind inside rock! We can fix in 1.20 I think, simply by checking for liquids at a higher point on the structure - but this will require testing and tuning. We don't want to lose the current behavior where vanilla ruins sometimes spawn underwater - it looks awesome in shallow lakes - but I can understand that surface ruins should not spawn deep underwater.

My current thinking: the liquid check should be at a random position between 3 and 7 blocks high on the structure: this will mean that structures will naturally be less and less likely to spawn, the deeper the water is, and nothing below depth 7. Something like that. Needs tuning.

@AreYSerious
Copy link
Contributor Author

Hey might be a stupid idea but why not just check 2 or 3 blocks +the offset of the structure. This would always check 2 or 3 blocks above ground level for water so the structure could spawn 1-2 blocks in water? @radfast
Just a fast idea haven't thought this though

@radfast
Copy link

radfast commented Feb 23, 2024

Yep, something like that, needs testing for the structures with very deep cellars.

Meanwhile, the "below mantle" issue, is that seen in 1.19.4-rc.1 or later? -rc.1 included code to fix structures attempting to generate off the map edge, I don't remember now whether we also check for below mantle

@AreYSerious
Copy link
Contributor Author

It's pre 1.19.4 and it wasn't reported directly by me but by one of the mod users.
The structure is currently disabled in the mod bcs of the prior performance issues and the mantle issue (which is rare since it only can happen to words with either oceans enabled or low Worldheight.

I can try to test it in the future but I'm generally not using oceans...

@radfast
Copy link

radfast commented Feb 25, 2024

I'll look at the mantle issue - though it may also be mitigatable using an appropriate minY value, see below.

You're right, the underwater generation issue will affect larger (wider) structures much more, as the 4 corners are more likely to be in rock/lake-bed not water. But it will affect vanilla and will need careful testing - we want vanilla ruins to be able to spawn still in shallow water - so a fix for that will have to wait until 1.20, I'm afraid.

Meanwhile you should be able to mitigate underwater generation completely by setting minY on the appropriate group of structures in the structures.json config. The default value is -0.3 if it is unset, I'm not sure why that default was chosen but in general we do allow surface worldgen (e.g. trees and grass) below sea level, to ensure that cave-mouths still have some vegetation - I don't really see the need for cave-mouths to have ruins, but there may be another edge case I have not thought of. In your mod, you can set minY to 0 or above for all your surface structures, that would force generation only above sea level.

@AreYSerious
Copy link
Contributor Author

Sounds good thanks for the detailed explanation! Will update that in my mod and give feedback if any issues come up

@AreYSerious
Copy link
Contributor Author

I messaged radfast about this on dc dm

The minY doesn't work at all atm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
department: code Issues apparently caused by code priority: medium Bugs and nuisances that need attention status: new This issue is fresh!
Projects
None yet
Development

No branches or pull requests

4 participants