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

Map generation limit: Fix checks for block/sector over-limit #4965

Closed
wants to merge 2 commits into from
Closed

Map generation limit: Fix checks for block/sector over-limit #4965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36f6b15
Map generation limit: Fix checks for block/sector over-limit
paramat Dec 27, 2016
542b4e1
Map generation limit: Cache as 'const' not 'const static'
paramat Jan 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 17 additions & 6 deletions src/map.cpp
View file
Open in desktop
Expand Up @@ -2064,15 +2064,26 @@ ServerMapSector *ServerMap::createSector(v2s16 p2d)
return sector;
}
#endif

/*
Do not create over-limit
Do not create over-limit.
We are checking for any nodes of the mapblocks of the sector being beyond the limit.
A sector is a vertical column of mapblocks, so sectorpos is like a 2D blockpos.

At the negative limit we are checking for
block minimum nodepos < -mapgenlimit.
At the positive limit we are checking for
block maximum nodepos > mapgenlimit.

Block minimum nodepos = blockpos * mapblocksize.
Block maximum nodepos = (blockpos + 1) * mapblocksize - 1.
*/
const static u16 map_gen_limit = MYMIN(MAX_MAP_GENERATION_LIMIT,
const u16 map_gen_limit = MYMIN(MAX_MAP_GENERATION_LIMIT,
g_settings->getU16("map_generation_limit"));
if(p2d.X < -map_gen_limit / MAP_BLOCKSIZE
|| p2d.X > map_gen_limit / MAP_BLOCKSIZE
|| p2d.Y < -map_gen_limit / MAP_BLOCKSIZE
|| p2d.Y > map_gen_limit / MAP_BLOCKSIZE)
if (p2d.X * MAP_BLOCKSIZE < -map_gen_limit
|| (p2d.X + 1) * MAP_BLOCKSIZE - 1 > map_gen_limit
|| p2d.Y * MAP_BLOCKSIZE < -map_gen_limit
|| (p2d.Y + 1) * MAP_BLOCKSIZE - 1 > map_gen_limit)
throw InvalidPositionException("createSector(): pos. over limit");

/*
Expand Down
33 changes: 22 additions & 11 deletions src/mapblock.h
View file
Open in desktop
Expand Up @@ -653,26 +653,37 @@ typedef std::vector<MapBlock*> MapBlockVect;

inline bool objectpos_over_limit(v3f p)
{
const static float map_gen_limit_bs = MYMIN(MAX_MAP_GENERATION_LIMIT,
const float map_gen_limit_bs = MYMIN(MAX_MAP_GENERATION_LIMIT,
g_settings->getU16("map_generation_limit")) * BS;
return (p.X < -map_gen_limit_bs
|| p.X > map_gen_limit_bs
|| p.X > map_gen_limit_bs
|| p.Y < -map_gen_limit_bs
|| p.Y > map_gen_limit_bs
|| p.Y > map_gen_limit_bs
|| p.Z < -map_gen_limit_bs
|| p.Z > map_gen_limit_bs);
|| p.Z > map_gen_limit_bs);
}

/*
We are checking for any node of the mapblock being beyond the limit.

At the negative limit we are checking for
block minimum nodepos < -mapgenlimit.
At the positive limit we are checking for
block maximum nodepos > mapgenlimit.

Block minimum nodepos = blockpos * mapblocksize.
Block maximum nodepos = (blockpos + 1) * mapblocksize - 1.
*/
inline bool blockpos_over_limit(v3s16 p)
{
const static u16 map_gen_limit = MYMIN(MAX_MAP_GENERATION_LIMIT,
const u16 map_gen_limit = MYMIN(MAX_MAP_GENERATION_LIMIT,
g_settings->getU16("map_generation_limit"));
return (p.X < -map_gen_limit / MAP_BLOCKSIZE
|| p.X > map_gen_limit / MAP_BLOCKSIZE
|| p.Y < -map_gen_limit / MAP_BLOCKSIZE
|| p.Y > map_gen_limit / MAP_BLOCKSIZE
|| p.Z < -map_gen_limit / MAP_BLOCKSIZE
|| p.Z > map_gen_limit / MAP_BLOCKSIZE);
return (p.X * MAP_BLOCKSIZE < -map_gen_limit
|| (p.X + 1) * MAP_BLOCKSIZE - 1 > map_gen_limit
Copy link
Member

Choose a reason for hiding this comment

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

why remove 1 from (p.X + 1) * MAP_BLOCKSIZE ? are you sure it's not (p.X + 1) * (MAP_BLOCKSIZE -1) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sure, see my working in first post, it does look weird though i know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should i add my maths working as a comment?

Copy link
Member

Choose a reason for hiding this comment

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

I think due to the complexity of the calcul and explanation in the code is a good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor Author

@paramat paramat Jan 6, 2017 •
edited

Choose a reason for hiding this comment

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

The min nodes of blocks are at (n * mapblocksize)
So the max nodes are at (n * mapblocksize) - 1
This is where the -1 comes from.

|| p.Y * MAP_BLOCKSIZE < -map_gen_limit
|| (p.Y + 1) * MAP_BLOCKSIZE - 1 > map_gen_limit
|| p.Z * MAP_BLOCKSIZE < -map_gen_limit
|| (p.Z + 1) * MAP_BLOCKSIZE - 1 > map_gen_limit);
}

/*
Expand Down