Created constant for lower bound of chunk #5488

12xx12 · 2023-04-08T20:36:25Z

Created new constant cChunkDef::LowerLimit
cChunkDef::Height -> cChunkDef::UpperLimit
replaced comparisons of absolute position and 0 with constant
Checks for height are now done with a function of cChunkDef constants

Name suggestions are welcome

Todo

Replace position comparisons with cChunkDef function

- Created new constant cChunkDef::BottomHeight - replaced comparisons of absolute position and 0 with constant

NiLSPACE · 2023-04-09T09:42:35Z

It might be useful to export some of these constants to the Lua api as well. Especially since the max and min chunk height was changed in 1.17. If plugins can use those constants now they wouldn't break when we increase/decrease the chunk limits.

- Created lua constants - renamed to upper and lower limit

CONTRIBUTING.md

src/Generating/Noise3DGenerator.cpp

12xx12 · 2023-04-11T14:19:14Z

I saw some other lines that need intervention

12xx12

Oopsie xD

src/Blocks/BlockLeaves.h

src/Entities/Entity.cpp

src/Generating/CompoGenBiomal.cpp

src/Map.cpp

src/Simulator/SimulatorManager.cpp

tests/Generating/BasicGeneratorTest.cpp

12xx12 · 2023-04-15T17:42:04Z

I am not sure if I got any occurence and if the change will break anything

madmaxoft · 2023-05-07T20:43:42Z

I think the cChunkDef::IsValidHeight should be refactored:

Drop the Vector3i version altogether (so the usage is the same as with cChunkDef::IsValidWidth)
Split it into two functions, one for single block, one for range, so that the range is more explicit at the call site

My reasoning for the split is that the following doesn't provide enough context to be checked in PRs:

if (cChunkDef::IsValidHeight(BlockY, 1))

What is that 1 there? Is that 1-block-above or 1-block-below?

Compare to this:

if (cChunkDef::IsValidHeightRange(BlockY, BlockY + 1))

It's clear at first sight what this is asking.

madmaxoft · 2023-05-07T20:28:38Z

Server/Plugins/APIDump/APIDesc.lua

+			{
+				Width =
+				{
+					Notes = "The width of a chunk, in number of blocks.",


Suggested change

Notes = "The width of a chunk, in number of blocks.",

Notes = "The X or Z size of a chunk, in number of blocks.",

madmaxoft · 2023-05-07T20:29:18Z

Server/Plugins/APIDump/APIDesc.lua

+				},
+				LowerLimit =
+				{
+					Notes = "The lower limit of the chunk coordinate vertical range.",


Suggested change

Notes = "The lower limit of the chunk coordinate vertical range.",

Notes = "The lower limit of the chunk coordinate vertical (Y) range.",

madmaxoft · 2023-05-07T20:29:52Z

Server/Plugins/APIDump/APIDesc.lua

+				},
+				UpperLimit =
+				{
+					Notes = "The upper limit of the chunk coordinate vertical range.",


Suggested change

Notes = "The upper limit of the chunk coordinate vertical range.",

Notes = "The upper limit of the chunk coordinate vertical (Y) range.",

madmaxoft · 2023-05-07T20:34:53Z

src/Entities/Entity.cpp

@@ -1011,7 +1011,7 @@ void cEntity::HandlePhysics(std::chrono::milliseconds a_Dt, cChunk & a_Chunk)
 	Vector3d NextPos = Vector3d(GetPosX(), GetPosY(), GetPosZ());
 	Vector3d NextSpeed = Vector3d(GetSpeedX(), GetSpeedY(), GetSpeedZ());

-	if ((BlockY >= cChunkDef::Height) || (BlockY < 0))
+	if ((BlockY >= cChunkDef::UpperLimit) || (BlockY < cChunkDef::LowerLimit))


Suggested change

if ((BlockY >= cChunkDef::UpperLimit) || (BlockY < cChunkDef::LowerLimit))

if (!cChunkDef::IsValidHeight(BlockY))

The off-by-one change introduced here doesn't matter much here I believe.

madmaxoft · 2023-05-07T20:36:17Z

src/ChunkDef.h

+	/** DEPRECATED!!! Validates a height-coordinate. Returns false if height-coordinate is out of height bounds.
+	Might be enclosed by a_SpaceBelow and a_SpaceAbove. [LowerLimit + a_SpaceBelow, UpperLimit - a_SpaceAbove) is valid.
+	@deprecated */
+	inline static bool IsValidHeight(int a_Height, int a_SpaceAbove = 0, int a_SpaceBelow = 0)
 	{
-		return ((a_BlockPosition.y >= 0) && (a_BlockPosition.y < Height));
+		ASSERT(a_SpaceBelow >= 0);
+		ASSERT(a_SpaceAbove >= 0);
+		return ((a_Height >= LowerLimit + a_SpaceBelow) && (a_Height < UpperLimit - a_SpaceAbove));
 	}


Why the deprecation? I think this is a perfectly fine usage of Y-coord only. After all, it's the same usage as IsValidWidth

madmaxoft · 2023-05-07T21:29:33Z

src/Generating/StructGen.cpp

 		{
 			FloorHi[INTERPOL_X * x + 17 * INTERPOL_Z * z] = (
 				m_Noise1.IntNoise3DInt(BaseX + INTERPOL_X * x, Segment + SEGMENT_HEIGHT, BaseZ + INTERPOL_Z * z) *
-				m_Noise2.IntNoise3DInt(BaseX + INTERPOL_Z * x, Segment + SEGMENT_HEIGHT, BaseZ + INTERPOL_Z * z) / 256
+				m_Noise2.IntNoise3DInt(BaseX + INTERPOL_Z * x, Segment + SEGMENT_HEIGHT, BaseZ + INTERPOL_Z * z) / cChunkDef::VerticalBlockCount


Suggested change

m_Noise2.IntNoise3DInt(BaseX + INTERPOL_Z * x, Segment + SEGMENT_HEIGHT, BaseZ + INTERPOL_Z * z) / cChunkDef::VerticalBlockCount

m_Noise2.IntNoise3DInt(BaseX + INTERPOL_Z * x, Segment + SEGMENT_HEIGHT, BaseZ + INTERPOL_Z * z) / 256

madmaxoft · 2023-05-07T21:29:49Z

src/Generating/StructGen.cpp

+				int Lo = FloorLo[x + (cChunkDef::Width + 1) * z] / cChunkDef::VerticalBlockCount;
+				int Hi = FloorHi[x + (cChunkDef::Width + 1) * z] / cChunkDef::VerticalBlockCount;


Suggested change

int Lo = FloorLo[x + (cChunkDef::Width + 1) * z] / cChunkDef::VerticalBlockCount;

int Hi = FloorHi[x + (cChunkDef::Width + 1) * z] / cChunkDef::VerticalBlockCount;

int Lo = FloorLo[x + (cChunkDef::Width + 1) * z] / 256;

int Hi = FloorHi[x + (cChunkDef::Width + 1) * z] / 256;

madmaxoft · 2023-05-07T21:32:43Z

src/LightingThread.cpp

@@ -261,7 +261,7 @@ void cLightingThread::LightChunk(cLightingChunkStay & a_Item)
 	{
 		for (int z = 0; z < cChunkDef::Width * 3; z++)
 		{
-			for (int y = cChunkDef::Height / 2; y >= 0; y--)
+			for (int y = cChunkDef::UpperLimit / 2; y >= cChunkDef::LowerLimit; y--)


Suggested change

for (int y = cChunkDef::UpperLimit / 2; y >= cChunkDef::LowerLimit; y--)

for (int y = cChunkDef::VerticalBlockCount / 2; y >= 0; y--)

madmaxoft · 2023-05-07T21:33:06Z

src/LightingThread.cpp

@@ -292,7 +292,7 @@ void cLightingThread::LightChunk(cLightingChunkStay & a_Item)
 	{
 		for (int z = 0; z < cChunkDef::Width * 3; z++)
 		{
-			for (int y = cChunkDef::Height / 2; y >= 0; y--)
+			for (int y = cChunkDef::UpperLimit / 2; y >= 0; y--)


Suggested change

for (int y = cChunkDef::UpperLimit / 2; y >= 0; y--)

for (int y = cChunkDef::VerticalBlockCount / 2; y >= 0; y--)

madmaxoft · 2023-05-07T21:37:28Z

src/LightingThread.cpp

@@ -357,7 +357,7 @@ void cLightingThread::PrepareSkyLight(void)
 	m_NumSeeds = 0;

 	// Fill the top of the chunk with all-light:
-	if (m_MaxHeight < cChunkDef::Height - 1)
+	if (cChunkDef::IsValidHeight(m_MaxHeight, 1))


❗ This needs rework, m_MaxHeight is used as an index into the internal array, so must be always 0-based, unlike an Y-coord.

madmaxoft · 2023-05-07T21:44:40Z

Good start, though I'm afraid it's still a long way from when we'll be able to use negative Y coords :)

Created constant for lower bound of chunk

8b63ebf

- Created new constant cChunkDef::BottomHeight - replaced comparisons of absolute position and 0 with constant

Exported to lua

2f3b04c

- Created lua constants - renamed to upper and lower limit

NiLSPACE reviewed Apr 10, 2023

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

Update docs

5ce8166

NiLSPACE reviewed Apr 10, 2023

View reviewed changes

src/Generating/Noise3DGenerator.cpp Outdated Show resolved Hide resolved

12xx12 commented Apr 11, 2023

View reviewed changes

12xx12 added 3 commits April 15, 2023 16:02

Replace occurances of comparisons with cChunkDef::Function

f91cc0f

Fixed bug introduced with constant

e117aa9

More constant replacements

0d8a207

Just dont ask me about that

11a5d45

madmaxoft requested changes May 7, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Created constant for lower bound of chunk #5488

Created constant for lower bound of chunk #5488

12xx12 commented Apr 8, 2023 •

edited

NiLSPACE commented Apr 9, 2023

12xx12 commented Apr 11, 2023

12xx12 left a comment

12xx12 commented Apr 15, 2023

madmaxoft commented May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft May 7, 2023

madmaxoft commented May 7, 2023

	Notes = "The width of a chunk, in number of blocks.",
	Notes = "The X or Z size of a chunk, in number of blocks.",

	Notes = "The lower limit of the chunk coordinate vertical range.",
	Notes = "The lower limit of the chunk coordinate vertical (Y) range.",

	Notes = "The upper limit of the chunk coordinate vertical range.",
	Notes = "The upper limit of the chunk coordinate vertical (Y) range.",

	if ((BlockY >= cChunkDef::UpperLimit) \|\| (BlockY < cChunkDef::LowerLimit))
	if (!cChunkDef::IsValidHeight(BlockY))

	m_Noise2.IntNoise3DInt(BaseX + INTERPOL_Z * x, Segment + SEGMENT_HEIGHT, BaseZ + INTERPOL_Z * z) / cChunkDef::VerticalBlockCount
	m_Noise2.IntNoise3DInt(BaseX + INTERPOL_Z * x, Segment + SEGMENT_HEIGHT, BaseZ + INTERPOL_Z * z) / 256

		int Lo = FloorLo[x + (cChunkDef::Width + 1) * z] / cChunkDef::VerticalBlockCount;
		int Hi = FloorHi[x + (cChunkDef::Width + 1) * z] / cChunkDef::VerticalBlockCount;

	for (int y = cChunkDef::UpperLimit / 2; y >= cChunkDef::LowerLimit; y--)
	for (int y = cChunkDef::VerticalBlockCount / 2; y >= 0; y--)

Created constant for lower bound of chunk #5488

Are you sure you want to change the base?

Created constant for lower bound of chunk #5488

Conversation

12xx12 commented Apr 8, 2023 • edited

Todo

NiLSPACE commented Apr 9, 2023

12xx12 commented Apr 11, 2023

12xx12 left a comment

Choose a reason for hiding this comment

12xx12 commented Apr 15, 2023

madmaxoft commented May 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madmaxoft commented May 7, 2023

12xx12 commented Apr 8, 2023 •

edited