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

Add a group that checks their distance to the ground on LOD-change #187

Closed
MathiasBaumgartinger opened this issue Feb 19, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@MathiasBaumgartinger
Copy link
Contributor

With assets/etc. that get instanced at a certain distance to the ground they soemtimes will end up being in the terrain once a new level of detail is loaded. As for the windmills and streets there is already a system that does something similar but with inheritance, a group would help making it more straightforward.

Look at GroundedSpatial and SpatialShift/PathShift for implementation.

@kb173
Copy link
Member

kb173 commented Apr 2, 2020

Starting to work on this because it'd be nice for the new streets - we could do work in threads more easily.

@kb173
Copy link
Member

kb173 commented Apr 2, 2020

After some consideration, the group approach may actually not be viable... The implementation heavily relies on reacting to position changes, which isn't possible with groups because _notification needs to be implemented in the node itself. We had a similar issue in the SpatialShift implementation, but that could be worked around by reacting to the SceneTree.node_added signal. Unfortunately, there's nothing like that for the Transform of a node changing...

With Geodot, a completely different solution would be viable: Not changing the height of assets at all, instead simply placing them at the precise location right at the start by getting the precise pixel value from the heightmap.

This does carry the assumption that the resolution of the terrain mesh is precise enough for the asset that is depicted, so it doesn't get underneath the ground. However, this should usually be correct because we won't display small assets where it matters at high distances with low LOD. With windmills, for example, I'd assume it's better if they don't change height when the terrain updates (it sticks out when focusing on the windmill) and are instead slightly within the ground.

Another thing that I feel like I noticed in some games is that small, high detail stuff like small rocks kind of comes out of the ground instead of popping in. This would help us with the issue of assets possibly floating because of a mismatch between heightmap resolution: The further they're from the camera, the larger their y-axis offset is. For large assets like windmills, this wouldn't be noticeable, but it could help with smaller things. The only situation where it could be bad is when moving at high speeds, causing the up-movement to be noticeable.

The approach described above would essentially decouple assets from the terrain tiles. The opposite solution would be to completely couple the two and to always have assets be children of their currently visible terrain tile. This would make it simple for them to get the ground position, but it would introduce a pretty big overhead and weird node tree shifting for all assets...

Done rambling now!

@kb173
Copy link
Member

kb173 commented Apr 2, 2020

@MathiasBaumgartinger When you have time, let me know what you think of these ideas - I think it's time to finally solve this properly 😉

@kb173
Copy link
Member

kb173 commented Dec 21, 2020

Related to the implementation of #242

@kb173
Copy link
Member

kb173 commented Jul 14, 2021

The ObjectRenderer places objects at the most precise known height value for a given location:

local_object_pos.y = layer.render_info.ground_height_layer.get_value_at_position(
		center[0] + local_object_pos.x, center[1] - local_object_pos.z)

This is consistent with my latest suggestion above, so no special group and height change polling is required. It's been implemented for a while and it seems to be working very well, so this issue can be closed.

@kb173 kb173 closed this as completed Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants