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.

Sign up for GitHub

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

Jump to bottom

Don't go out of the map during raycast #11831

Closed
wants to merge 3 commits into from
Closed

Don't go out of the map during raycast #11831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6164a0e
Don't go out of the map during raycast
savilli Dec 4, 2021
46c2a83
More paranoia
savilli Dec 6, 2021
c4371f5
Update
savilli Dec 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/environment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,56 @@ inline static bool isPointableNode(const MapNode &n,
(liquids_pointable && features.isLiquid());
}

// Find such i that for any j overlap[i] / extent[i] > overlap[j] / extent[j],
// or overlap[i] * extent[j] > extent[i] * overlap[j]
inline static void updateMaxOverlap(f32 overlap, f32 extent, f32& max_overlap, f32& max_overlap_extent)
{
if (overlap > 0 && (max_overlap_extent == 0 || overlap * max_overlap_extent > extent * max_overlap)) {
SmallJoker marked this conversation as resolved.
Show resolved Hide resolved
max_overlap = overlap;
max_overlap_extent = extent;
}
}

void Environment::continueRaycast(RaycastState *state, PointedThing *result)
{
const NodeDefManager *nodedef = getMap().getNodeDefManager();
if (state->m_initialization_needed) {
// Check boundaries
const float limit = (MAX_MAP_GENERATION_LIMIT + 0.5f) * BS;
if (!std::isfinite(state->m_shootline.start.X) ||
!std::isfinite(state->m_shootline.start.Y) ||
!std::isfinite(state->m_shootline.start.Z) ||
!std::isfinite(state->m_shootline.end.X) ||
!std::isfinite(state->m_shootline.end.Y) ||
!std::isfinite(state->m_shootline.end.Z) ||
Copy link
Member

Choose a reason for hiding this comment

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

this is already assured

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where?

Copy link
Member

@SmallJoker SmallJoker Dec 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Where?

Isn't this covered by std::abs(state->m_shootline.end.Y) > 1000000 * limit below? Infinity is always greater than limit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about NaN?

Copy link
Member

Choose a reason for hiding this comment

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

What about std::isnan to differentiate those cases easily in code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

std::isnan makes code less clear IMO

Copy link
Contributor

@appgurueu appgurueu Jan 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Wrong place to catch this IMO. We expect the engine to only pass finite numbers. And the Lua side of things should get an error thrown if it passes infinities or NaN, not just get no pointed things.

std::abs(state->m_shootline.start.X) > limit ||
std::abs(state->m_shootline.start.Y) > limit ||
std::abs(state->m_shootline.start.Z) > limit) {
Comment on lines +137 to +139
Copy link
Contributor

@appgurueu appgurueu Jan 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't like how this silently aborts if the start is out of bounds. It should do the same clamping as is done for the end.

// Abort, no collisions
result->type = POINTEDTHING_NOTHING;
return;
}

v3f extent = state->m_shootline.getVector();
f32 max_overlap = 0;
f32 max_overlap_extent = 0;

// Get the biggest linear fraction of the overlap within the extent cuboid
updateMaxOverlap(std::abs(state->m_shootline.end.X) - limit, std::abs(extent.X), max_overlap, max_overlap_extent);
updateMaxOverlap(std::abs(state->m_shootline.end.Y) - limit, std::abs(extent.Y), max_overlap, max_overlap_extent);
updateMaxOverlap(std::abs(state->m_shootline.end.Z) - limit, std::abs(extent.Z), max_overlap, max_overlap_extent);

if (max_overlap_extent > 0)
state->m_shootline.end -= extent * max_overlap / max_overlap_extent; // Move the extent backwards

// Fix possible overflows due to rounding errors
state->m_shootline.end.X = core::clamp(state->m_shootline.end.X, -limit, limit);
state->m_shootline.end.Y = core::clamp(state->m_shootline.end.Y, -limit, limit);
state->m_shootline.end.Z = core::clamp(state->m_shootline.end.Z, -limit, limit);

state->m_iterator = voxalgo::VoxelLineIterator(state->m_shootline.start / BS, state->m_shootline.getVector() / BS);
state->m_previous_node = state->m_iterator.m_current_node_pos;

// Add objects
if (state->m_objects_pointable) {
std::vector<PointedThing> found;
Expand Down
5 changes: 1 addition & 4 deletions src/raycast.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,9 @@ bool RaycastSort::operator() (const PointedThing &pt1,
RaycastState::RaycastState(const core::line3d<f32> &shootline,
bool objects_pointable, bool liquids_pointable) :
m_shootline(shootline),
m_iterator(shootline.start / BS, shootline.getVector() / BS),
m_previous_node(m_iterator.m_current_node_pos),
m_objects_pointable(objects_pointable),
m_liquids_pointable(liquids_pointable)
{
}
{}


bool boxLineCollision(const aabb3f &box, const v3f &start,
Expand Down
6 changes: 4 additions & 2 deletions src/voxelalgorithms.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ struct VoxelLineIterator
//! Position of the start node.
v3s16 m_start_node_pos;
//! Index of the last node
s16 m_last_index;
s16 m_last_index = 0;

VoxelLineIterator() {}

/*!
* Creates a voxel line iterator with the given line.
Expand All @@ -126,7 +128,7 @@ struct VoxelLineIterator
* line in voxel coordinates. start_position+line_vector
* is the end of the line
*/
VoxelLineIterator(const v3f &start_position,const v3f &line_vector);
VoxelLineIterator(const v3f &start_position, const v3f &line_vector);

/*!
* Steps to the next voxel.
Expand Down