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

Biomes: Add X and Z biome limits / Getv3intfield: Fix logic of return bool #7070

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
5 participants
@paramat
Copy link
Member

commented Feb 23, 2018

Commit 1:
Biomes: Add X and Z biome limits

Commit 2:
Getv3intfield: Fix logic of return bool

Commit 3:
Biomes: Document alternative xyz biome limits
////////////////

screenshot_20180223_094403

^ Normal biomes restricted to (-100, -100) to (100, 100)

For part of #5725
Set limits are precise to a node.
Backwards compatible with any biome registration using 'y_min' and 'y_max'.
Uses the new 'getv3intfield' feature, which commit 2 fixes.

Testing mod:

minetest.clear_registered_biomes()
minetest.clear_registered_ores()
minetest.clear_registered_decorations()

	minetest.register_biome({
		name = "upper",
		--node_dust = "",
		node_top = "default:dirt_with_grass",
		depth_top = 1,
		node_filler = "default:dirt",
		depth_filler = 3,
		node_stone = "default:stone",
		--node_water_top = "",
		--depth_water_top = ,
		--node_water = "",
		--node_river_water = "",
		min_pos = {x = -100, y = -100, z = -100},
		max_pos = {x = 100, y = 100, z = 100},
		heat_point = 50,
		humidity_point = 50,
	})

@paramat paramat force-pushed the paramat:xyzbiomelim branch from 74a6f25 to 2b9d19c Feb 23, 2018

@paramat paramat removed the WIP label Feb 23, 2018

@paramat paramat force-pushed the paramat:xyzbiomelim branch from 2b9d19c to e81cee1 Feb 23, 2018

@CoderForTheBetter

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2018

@paramat Looks great! I'll mess around with this pull request later, but maybe it is possible for you to answer this. In the above, you say that you restricted the normal biomes, so my question is if you restrict a single biome to the parameters you used, will other biomes fill in the places that the restricted biome would have spawned? My worry is that there will be bare stone areas if this feature is used.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Feb 23, 2018

If you restrict biome 'A' to an area, within that area all biomes are possible, outside the area all biomes except A are possible, so no bare stone outside the area.
To be clear, within the area, biome A will not override all others, which biome appears depends on heat, humidity and biome points.

@paramat paramat closed this Feb 23, 2018

@paramat paramat reopened this Feb 23, 2018

@paramat

This comment has been minimized.

Copy link
Member Author

commented Feb 23, 2018

(Wrong button).

Build fail is travis error.

x_max = 31000,
z_min = -31000,
z_max = 31000,
-- ^ Horizontal limits for biome. min/max default to -/+31000.

This comment has been minimized.

Copy link
@SmallJoker

SmallJoker Feb 24, 2018

Member

Having these as two vectors, minp and maxp would be more readable as the coordinates can be compared easily vertically between the two code lines.

This comment has been minimized.

Copy link
@paramat

paramat Feb 24, 2018

Author Member

Yes i considered that, i chose this way for backwards compatibility and because for the majority of the time only y_min and y_max will be used, vectors would always require X and Z to be written out whereas this way those can be forgotten and left to defaults.

This comment has been minimized.

Copy link
@SmallJoker

SmallJoker Feb 24, 2018

Member

In that case, a new getv3s16field_default would be handy. Values like minp = { y = 0, z = -1000 }, maxp = { y = 100, z = 100 } could be used in that case. Not set coordinate values would have a default value of 31000, -31000 respectively.

This comment has been minimized.

Copy link
@paramat

paramat Feb 24, 2018

Author Member

Hmm not sure, 95-99% of the time only 'y_min' and 'y_max' will be used in biome registrations.
Would your suggestion accept simple 'y_min' and 'y_max'?

This comment has been minimized.

Copy link
@SmallJoker

SmallJoker Feb 24, 2018

Member

Yes, if there's a line of compatibility code, which would be needed anyway if switching to a vector value: (untested)

b->minp = getv3s16field_default(L, index, "minp", v3s16(-31000));
getintfield(L, index, "y_min", b->minp.Y); // Overwrite if existing
b->maxp = getv3s16field_default(L, index, "maxp", v3s16(31000));
getintfield(L, index, "y_max", b->maxp.Y); // Overwrite if existing

This comment has been minimized.

Copy link
@paramat

paramat Feb 24, 2018

Author Member

Ok something to consider, PR is now WIP.

s16 y_min;
s16 y_max;
s16 z_min;
s16 z_max;

This comment has been minimized.

Copy link
@SmallJoker

SmallJoker Feb 24, 2018

Member

Connected with the last comment, these here could also be replaced by two v3s16 vectors.

@paramat paramat added the WIP label Feb 24, 2018

@rubenwardy

This comment has been minimized.

Copy link
Member

commented Feb 24, 2018

If the X and Z limits are reasonably large and alternative biomes exist, will straight edges and corners be avoided?

@paramat

This comment has been minimized.

Copy link
Member Author

commented Feb 24, 2018

In that situation straight edges cannot be avoided. This feature is quite specialist in it's uses, my use case is for cubic planets floating in space, each having it's own biome system, in this case the biomes being confined to cuboids is not a problem. There are probably other uses but it is not for smoothly blending a biome into a confined area.
The screenshot is just a test and not a use case.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2018

Hmm i'm unsure about this now, it's only for a certain mapgen idea i have that may not happen. Hopefully others can explain more use cases for this.
I think definable biome air nodes is more important.

@CoderForTheBetter

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2018

@paramat, I'll give it a go. My idea is to use it to make a mod that as the player heads a certain direction the biomes get tougher and tougher to live in. There could be a mod where it tells the player to head to a certain area (small biome constrained by x and z coordinates). It would also make it very easy for arena type worlds/maps to be generated without as much work (think procedural maps for ctf without using the voxel manipulator).

There's still your idea and the ideas of others, but it is not possible for me to think of every use case.

I know you guys don't want to add features that are not used, but I feel like this feature has the potential for many different uses.

Also, mapgen is my favorite thing to mess around with, and I remember the first time I made a mod I went directly to look to see if I could constrain the x and z of a biome, and was very disappointed when finding out that I could not. It felt obvious that it should be possible.

Anyway, I hope I convinced you, otherwise I understand if you don't want to add it if you feel like it is a useless feature. I was just hoping that it could make the 0.5.0 release. The sooner it is added, the better.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2018

Don't worry i'm not opposed at all, and this is quite simple, it will certainly have some interesting uses. I'm happy to go ahead if other devs like the idea.

@paramat paramat force-pushed the paramat:xyzbiomelim branch from e81cee1 to 37ef15d Feb 26, 2018

@paramat

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2018

SmallJoker i agree, will make this accept an integer or a v3s16.

@paramat paramat force-pushed the paramat:xyzbiomelim branch 2 times, most recently from a350cbc to f9e6cc9 Feb 27, 2018

@paramat

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2018

SmallJoker updated to your suggestion but needs the new 'getv3s16field (default)' functions, which i'm finding difficult.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 3, 2018

#7090 merged so i will update this.

@paramat paramat force-pushed the paramat:xyzbiomelim branch from f9e6cc9 to 727386b Mar 3, 2018

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 3, 2018

@SmallJoker is the form of input

		min_pos = {x = -100, y = -100, z = -100},
		max_pos = {x = 100, y = 100, z = 100},

?
This is not working for me, seems to be falling back to the default.

@SmallJoker

This comment has been minimized.

Copy link
Member

commented Mar 3, 2018

@paramat Stupid me, didn't check whether the function works as expected. Below is a fix that 1) tries all coordinates and 2) uses the correct Lua index.

diff --git a/src/script/common/c_converter.h b/src/script/common/c_converter.h
index 67b23f1..04fdb35 100644
--- a/src/script/common/c_converter.h
+++ b/src/script/common/c_converter.h
@@ -66,9 +66,9 @@ bool getv3intfield(lua_State *L, int index,
        lua_getfield(L, index, fieldname);
        bool got = false;
        if (lua_istable(L, -1)) {
-               got = getintfield(L, index, "x", result.X) ||
-                       getintfield(L, index, "y", result.Y) ||
-                       getintfield(L, index, "z", result.Z);
+               got |= getintfield(L, -1, "x", result.X);
+               got |= getintfield(L, -1, "y", result.Y);
+               got |= getintfield(L, -1, "z", result.Z);
        }
        lua_pop(L, 1);
        return got;
@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2018

@SmallJoker do you have time to merge that fix? If not i can do it.

@SmallJoker

This comment has been minimized.

Copy link
Member

commented Mar 4, 2018

@paramat Or include it in this PR, as the function has no use for it yet and would be required for this feature anyway 8)

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2018

Ok i'll do it.

@paramat paramat removed the WIP label Mar 4, 2018

@paramat paramat changed the title Biomes: Add X and Z biome limits Biomes: Add X and Z biome limits / Getv3intfield: Fix logic of return bool Mar 4, 2018

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2018

Updated with 2nd commit. Seems to work but will do some more testing.

@CoderForTheBetter

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2018

I did a quick test:
selection_027

Everything seems to work fine, to me at least.

@paramat paramat force-pushed the paramat:xyzbiomelim branch from 514ef66 to 5f7dea5 Mar 7, 2018

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2018

Ok i'll do a quick test then it can be merged.

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2018

Testing seems ok, i tested specifying x or z only and the default values are used for the axes left out, for example:

		min_pos = {x = -100},
		max_pos = {x = 100},

WIP need to add a 3rd commit for docs.

@paramat paramat added WIP and removed WIP labels Mar 7, 2018

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2018

Added docs.

paramat added some commits Feb 23, 2018

@paramat paramat force-pushed the paramat:xyzbiomelim branch from 0e51063 to 029234f Mar 8, 2018

@paramat paramat force-pushed the paramat:xyzbiomelim branch from 029234f to 780cd0a Mar 8, 2018

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 9, 2018

@paramat paramat closed this Mar 9, 2018

@twoelk

This comment has been minimized.

Copy link

commented Mar 15, 2018

just wondering wether this could be used to read biome extends from some external input source. (well you would have a mod reading those sources - I guess)
Or maybe some biome defining information such as a custom made heatmaps overriding normal mapgen in defined areas.

Consider for example the map of OZ - a central world surrounded by a ring of deserts, with normal biomes beyond that.

Besides from that I strongly hope for at least some sort of fuzzy border option added to this , some day.

hm, will this be the way to add biomes to the underground?

@paramat

This comment has been minimized.

Copy link
Member Author

commented Mar 15, 2018

wondering wether this could be used to read biome extends from some external input source

No, however there is an issue requesting the ability to define custom heat / humidity maps that is used by core mapgen.

We can already add biomes underground.

@paramat paramat deleted the paramat:xyzbiomelim branch Mar 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.