Time |
Nick |
Message |
04:00 |
|
MTDiscord joined #minetest-hub |
05:32 |
|
NathanS21 joined #minetest-hub |
08:00 |
|
ShadowNinja joined #minetest-hub |
08:15 |
|
hlqkj joined #minetest-hub |
08:46 |
|
olliy joined #minetest-hub |
09:34 |
|
calcul0n_ joined #minetest-hub |
09:54 |
VanessaE |
so, I've been over nearly every byte of biome_lib code, rewriting a few things here and there, and still can't figure out what's causing occasional empty mapblocks. |
09:55 |
VanessaE |
can someone please take a look? (https://gitlab.com/VanessaE/biome_lib/-/blob/master/init.lua) |
10:57 |
|
Fixer joined #minetest-hub |
11:00 |
|
calcul0n__ joined #minetest-hub |
11:58 |
|
Fixer joined #minetest-hub |
13:23 |
MTDiscord |
<Warr1024> What's the deal with using biome_lib:method()s instead of biome_lib.method()s when you reference buome_lib but not self? |
13:23 |
VanessaE |
old. |
13:23 |
VanessaE |
that's all |
13:24 |
VanessaE |
I've been changing over the latter as I make little reworks here and there |
13:24 |
VanessaE |
but mostly the colon notation is simply because it's old :) |
13:24 |
VanessaE |
when I started it, I wasn't all that good at Lua |
13:28 |
MTDiscord |
<Warr1024> Hmm, I also make some methods dual local/library, i.e the ones I want to share with dependents but not necessarily allow them to override. That way I can reference the local one internally, which is not only faster at runtime, but more importantly easier to read without the library name plastered everywhere... |
13:28 |
MTDiscord |
<Warr1024> Always takes a while to get past stylistic things before I can start to actually understand the logic. |
13:29 |
VanessaE |
right |
13:30 |
VanessaE |
I think I'll finish-up transitioning it all over to period notation with my next set of changes (and just provide shims for old mods) |
13:30 |
MTDiscord |
<Warr1024> Does this thing really infer the types of nodes that can be on the surface based on the first mapblock it sees actually generated? |
13:30 |
VanessaE |
no |
13:31 |
MTDiscord |
<Warr1024> Trying to figure out what https://gitlab.com/VanessaE/biome_lib/-/blob/master/init.lua#L435 is all about |
13:31 |
VanessaE |
each block that's emerged by the mapgeg gets logged, scanned, populated, and then removed from the log |
13:31 |
VanessaE |
mapgen* |
13:32 |
VanessaE |
I'm using the position hash as a key to a table that holds everything having to do with the mapblock at that position |
13:34 |
VanessaE |
the idea being that each mapblock gets a full workup before it's removed from the block log, then on to the next |
13:35 |
MTDiscord |
<Warr1024> Oh, you can't use a local because you do this across multiple ticks? That's kinda convoluted... |
13:36 |
VanessaE |
yeah |
13:37 |
VanessaE |
it has to be spread-out in order to not lag |
13:37 |
MTDiscord |
<Warr1024> I don't think it pays to be too "polite" and split work up too much across multiple ticks, unless you pay attention to dtime and consider that other mods may not be as polite back to you. If dtime creeps up but you still do only a tiny amount of work your queue can end up relatively starved... |
13:37 |
VanessaE |
I do. |
13:37 |
VanessaE |
https://gitlab.com/VanessaE/biome_lib/-/blob/master/init.lua#L490 |
13:37 |
VanessaE |
as you can see, it can be configured to run multiple steps per tick |
13:37 |
MTDiscord |
<Warr1024> Feels like this function could have been more readable as a coro. |
13:38 |
MTDiscord |
<Warr1024> Nothing exposes every little code readability problem quite like trying to read it on a phone screen. |
13:38 |
VanessaE |
haha |
13:38 |
VanessaE |
I can't do too much about that :S |
13:38 |
VanessaE |
:D |
13:39 |
MTDiscord |
<Warr1024> Oh wait, your dtime check is backwards. |
13:39 |
MTDiscord |
<Warr1024> If dtime is larger, you do less work, not more |
13:41 |
MTDiscord |
<Warr1024> You do a fixed number of entries per step but if some other mod is eating up time, biome_lib basically freezes itself until that mod quiesces, but there's no guarantee it ever will. |
13:42 |
MTDiscord |
<Warr1024> I guess that kind of behavior shouldn't be unexpected considering how you run your servers... |
13:42 |
VanessaE |
huh |
13:43 |
VanessaE |
how should I do it then? the way dtime works has always confused me |
13:43 |
VanessaE |
I always thought it was the real amount of time the last server tick used |
13:44 |
VanessaE |
i.e. if it's getting higher, mods are (in general) sucking-up server time |
13:44 |
celeron55 |
it's the amount of time since the last tick (or, well, i guess MT usually calls them steps) |
13:45 |
celeron55 |
if it increases from the set amount, that means there's overload |
13:45 |
VanessaE |
the idea in my case being that biome_lib should step out of the way if ticks are taking too long |
13:45 |
MTDiscord |
<Warr1024> Yes. It represents the amount of time you need to catch up on. Unfortunately, managing it is fairly complex. If you're too polite then you can starve, but if you're too aggressive you can cause a meltdown. |
13:46 |
MTDiscord |
<Warr1024> NodeCore optics are supposed to run on a fixed 12Hz timing. I do as many iterations as dtime tells me I should ... but I have a hard limit to ensure optics themselves don't cause a meltdown. |
13:46 |
celeron55 |
i'd say it's best to neither try to catch up nor step out of the way |
13:47 |
VanessaE |
so in the case of this code, perhaps I should just drop the dtime check and keep the run ratio and per-step features |
13:47 |
MTDiscord |
<Warr1024> Also stepping out of the way doesn't necessarily help since the expensive tick has already happened anyway... |
13:48 |
VanessaE |
good point. |
13:48 |
MTDiscord |
<Warr1024> Does the run ratio really help all that much either? Might as well just run every tick, if there is work to do ... You've already done a lot to keep the per-tick work level down. |
13:49 |
MTDiscord |
<Warr1024> Just spreading the workload out across multiple ticks is already a great lag-killer, and it's hard to substantively improve on that without risking doing more harm than good. |
13:49 |
celeron55 |
it seems the amount of work to be done is already set and what needs to be done is smooth it out as much as possible, but never to not do it |
13:50 |
VanessaE |
I put it there for really slow machines, though on my PC and servers, I don't use it |
13:50 |
MTDiscord |
<Warr1024> On really slow machines, biome_lib would be so polite to other mods that gameplay would be impacted and players could see uninitialized blocks. |
13:51 |
MTDiscord |
<Warr1024> Though I don't think that's the cause of what we're seeing... |
13:51 |
VanessaE |
nopw |
13:51 |
MTDiscord |
<Warr1024> Btw, I don't see where you're doing your neighboring block checks... |
13:51 |
VanessaE |
even when it comes up with these empty blocks, the block list does eventually run dry once I stop running around |
13:52 |
VanessaE |
oh that, I split the neighboring-blocks check into a different branch |
13:52 |
VanessaE |
to be explored later |
13:53 |
VanessaE |
so this'll be the new globalstep callback then: https://pastebin.com/rEfX6c84 |
13:55 |
VanessaE |
celeron55: did you see the root prob we're trying to solve though? |
13:57 |
celeron55 |
i did notice, don't have any insight to it |
13:57 |
VanessaE |
ok. |
13:58 |
celeron55 |
but given how your system works, maybe the mapgen sometimes overwrites your blocks? |
13:58 |
VanessaE |
that's what I'm wondering |
13:59 |
celeron55 |
as far as i'm aware it will read and when it's done then overwrite the bordering mapblocks of the generated chunk |
13:59 |
VanessaE |
but if it's overwriting blocks altered *after* they're emerged, isn't that a bug? |
14:00 |
|
Peppy joined #minetest-hub |
14:00 |
celeron55 |
decorations and some other stuff extend over the chunk, and they're allowed one mapblock |
14:01 |
celeron55 |
and as it's in a different thread using its own buffer it doesn't have any idea what's happening meanwhile |
14:01 |
celeron55 |
unless i fail to remember a way it was made to avoid this |
14:02 |
VanessaE |
I guess I need to call upon the mapgen to always emerge all 6 neighbors of a given chunk before logging it? |
14:02 |
celeron55 |
so effectively the border mapblocks are half-generated until the neighboring chunk is generated |
14:02 |
VanessaE |
(using the emerge area call) |
14:03 |
celeron55 |
but i don't think that status is available to anything other than the emerge thread |
14:04 |
celeron55 |
as a workaround, maybe, but ideally there would be some kind of events or flags or something to allow knowing when something is actually fully done |
14:04 |
celeron55 |
also still i'm still not sure i remember right |
14:05 |
celeron55 |
maybe it's already handled smehow |
14:05 |
celeron55 |
+o |
14:05 |
VanessaE |
I remember there being a one-block "grey area" around a chunk for mud flow but that's pretty old now, and I don't recall that deleting/overwriting the border areas |
14:23 |
VanessaE |
if I call `minetest.emerge_area()`, will it in turn trigger the regular `on_generated()`? |
14:24 |
sfan5 |
if the chunk(s) do not already exist it will trigger the mapgen and everything it usually does |
14:28 |
MTDiscord |
<Warr1024> You have to accept that some blocks must be able to be in a half-generated state though, because if you try to emerge the neighboring blocks whenever these happen, you'll just effectively recursively queue up emergence of the entire universe. |
14:28 |
VanessaE |
but how can I *detect* this "half-generated" state? |
14:36 |
MTDiscord |
<Warr1024> Haha |
14:36 |
MTDiscord |
<Warr1024> I'd be surprised if you actually can... |
14:36 |
MTDiscord |
<Warr1024> You can tell when a block MIGHT be half-generated based on the fact that at least one neighbor isn't emerged |
14:37 |
VanessaE |
I tried that. |
14:37 |
MTDiscord |
<Warr1024> if the neighbor is emerged it shouldn't be able to be in a half-assed state |
14:37 |
VanessaE |
check the extra-map-checks branch, you can see what I did. |
14:37 |
VanessaE |
it didn't work |
14:37 |
VanessaE |
the check for neighbors happens too quickly |
14:38 |
MTDiscord |
<Warr1024> about the only thing I can think of beyond that is to (1) impose an artificial delay to give the worker thread a chance to win the race, and then (2) pray that whatever arbitrary value you picked is sufficient to work but not so bad that a fast-flying player could see behind the curtain. |
14:38 |
VanessaE |
it's like the core mapgen falls "behind" the Lua side |
14:38 |
MTDiscord |
<Warr1024> I absolutely abhor fine-tuning problems, but without some way to properly check the block state... |
14:39 |
MTDiscord |
<Warr1024> unfortunately I don't think that blocks themselves are aware of half-generated-ness, nor does the engine likely actually track this; it's probably just an emergent state. |
14:41 |
MTDiscord |
<Warr1024> I really don't see how my neighbor mapblock check should have failed entirely, though ... unless the game thinks that those mapblocks are generated and all ignores/air because they were included in the boundary region of the mapblock you're checking. |
14:41 |
MTDiscord |
<Warr1024> Maybe (1) either check for any ignores instead of just nil, or (2) try extending the check to 2 mapblocks around. |
14:42 |
MTDiscord |
<Warr1024> (2) might be the most reliable given that hypothesis. |
14:42 |
VanessaE |
minetest.get_node_or_nil() returns nil if it gets an "ignore" node. |
14:42 |
MTDiscord |
<Warr1024> No it doesn't |
14:42 |
VanessaE |
no?> |
14:42 |
MTDiscord |
<Warr1024> it returns nil if the node is not loaded |
14:42 |
VanessaE |
https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L4875 |
14:42 |
VanessaE |
yeah, not loaded == ignore |
14:43 |
MTDiscord |
<Warr1024> no, ignores are not just for not loaded areas |
14:43 |
MTDiscord |
<Warr1024> they're also for areas that have ignores |
14:43 |
MTDiscord |
<Warr1024> you can have ignores explicitly set in an actually-loaded mapblock |
14:43 |
VanessaE |
wat |
14:44 |
VanessaE |
https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L4871-L4876 this disagrees with you |
14:44 |
MTDiscord |
<Warr1024> The thought here is that this might be how the engine handles blocks that aren't generated yet but already have structures intruding into them from neighboring mapgen. |
14:44 |
MTDiscord |
<Warr1024> Huh, never noticed there was a bulk_set_node call ... but I don't see what that has to do with ignores. |
14:46 |
VanessaE |
that might be something worth looking into later. |
14:47 |
MTDiscord |
<Warr1024> Well, you can easily test whether get_node_or_nil ever returns an ignore. You could even just patch the API func itself to throw a log message. |
14:51 |
MTDiscord |
<Warr1024> Besides, what would be the point of having both get_node with ignores and get_node_or_nil if they were exactly equal? |
14:52 |
VanessaE |
I imagine so that you can do stuff like, `if minetest.get_node_or_nil(pos) then...` without checking for a node name |
14:53 |
VanessaE |
i.e. a bool versus string test |
14:53 |
MTDiscord |
<appguru> ^ |
14:53 |
VanessaE |
I guess string handling/comparisons are slightly slower than bools in Lua |
14:53 |
MTDiscord |
<appguru> It's more about convenience I'd say |
14:54 |
MTDiscord |
<Warr1024> Okay, I just did the empirical test and proved that get_node_or_nil can return an ignore nodes |
14:54 |
MTDiscord |
<appguru> String comparison in Lua is constant time usually |
14:54 |
MTDiscord |
<Warr1024> So all debate about why they would do a thing they didn't do is moot |
14:54 |
VanessaE |
Warr: wait what? |
14:54 |
MTDiscord |
<appguru> ah yes, minetest at it again... |
14:54 |
MTDiscord |
<Warr1024> I think the problem here is that mapblocks can't be in a half-generated state |
14:54 |
MTDiscord |
<Warr1024> they can actually be in a 1/3-generated and 2/3-generated state |
14:55 |
MTDiscord |
<Benrob0329> Yeah, its not a guaranteed function iirc |
14:55 |
MTDiscord |
<Warr1024> 2/3 meaning they have terrain but not necessarily structures from neighbors, and 1/3 meaning they have ONLY structures from neighbors but no terrain yet. |
14:55 |
MTDiscord |
<Warr1024> get_node_or_nil exists to differentiate between nodes that aren't loaded and nodes that are ignores, i.e. not initialized |
14:56 |
MTDiscord |
<Warr1024> and apparently nodes can be loaded and initialized in either order |
14:56 |
MTDiscord |
<Warr1024> I'm not sure why mapblocks are allowed to be emerged when they aren't finished yet, but ? |
14:57 |
VanessaE |
so you're suggesting, what, check if or_nil().name == "ignore" and call that "not fully emerged"? |
14:57 |
MTDiscord |
<Warr1024> Expanding the nil check to also include ignores (or really just using get_node and ignores) would SORTA work, unless you happened to unluckily pick a node to test in the neighboring mapblock that had content in it spilled over from the loaded area. You could check mulitple, but there's still no absolute guarantee and that's expensive. |
14:57 |
MTDiscord |
<Warr1024> So better would be to just test 2 mapblocks away, since I don't think the spillover region is allowed to be larger than 1 mapblock... |
14:58 |
MTDiscord |
<Warr1024> _or_nil() ~= nil and _or_nil().name == "ignore" could test for at least this half-emerged case, but it'd be too spotty to rely on; spottiness is itself what we're trying to fix here, so that'd be a mitigation, not a fix. I say we shoot for a fix first, and fall back to a mitigation if we can't actually fix it. |
14:59 |
MTDiscord |
<Warr1024> just check 32 nodes away instead of 16 and retest. Code change should be simple, and there's a chance it will work. |
14:59 |
|
aerozoic joined #minetest-hub |
14:59 |
MTDiscord |
<Warr1024> If it DOES work, then we can stand around debating why it works and whether it reasonably should, but at least we can do it with biome_lib being likely free from one more bug :-) |
15:00 |
VanessaE |
haha |
15:00 |
MTDiscord |
<Warr1024> If you hate hacky workarounds then the minetest universe has some bad news for you :-/ |
15:00 |
VanessaE |
hacky workarounds are fine if they're reliable and cheap. |
15:01 |
VanessaE |
(cheap == doesn't gulp down CPU, in this case |
15:01 |
VanessaE |
) |
15:01 |
MTDiscord |
<Warr1024> You always need some kinda hacky stuff when you're dealing with edge-cases. The only real alternative is to avoid the edge cases, and sometimes all that really does is move the edge inwards. |
15:02 |
MTDiscord |
<Warr1024> In effect we're sort of doing that, because you now won't try to run the mapblocks on the actual edge, creating a new edge of engine-initialized-but-not-biomelib-initialized blocks |
15:02 |
MTDiscord |
<Warr1024> they'll be farther in from the mapgen boundary, but hopefully 1 more mapblock in isn't enough to affect the experience |
15:02 |
MTDiscord |
<Warr1024> On the plus side, you should have eventual consistency restored. |
15:08 |
VanessaE |
well c55 did say earlier that the maximum overlap is 1 mapblock, so poking at the middle of a chunk is guaranteed to never be "half emerged" |
15:17 |
VanessaE |
there's another corner case though: a chunk out at the edge of the player distance limit. generated in full, but then the next one out past is not generated at all. suppose the fully-generated one has already been "plantified". what happens to it when the player walks forward, and that slightly more distant mapblock finally generates? |
15:17 |
VanessaE |
more distant chunk I mean. |
15:18 |
VanessaE |
or rather, what happens to the overlap area |
15:33 |
VanessaE |
Warr: https://pastebin.com/vs8E760R something like this? (scroll to line 427 of the paste) |
15:35 |
VanessaE |
and... fail. |
15:38 |
VanessaE |
well not utter fail |
15:39 |
VanessaE |
what was long strips of unpopulated terrain is now a single mapblock here or there. and the debug output shows it IS detecting not-loaded neighbors |
15:41 |
MTDiscord |
<Benrob0329> Is it possible that this issue doesn't exist for users of a VoxelManip because that forces Minetest to emerge the area into a buffer? |
15:42 |
MTDiscord |
<Benrob0329> Maybe either call emerge, or use the LVM equivalents of get_ and set_node? |
15:42 |
VanessaE |
mmmh |
15:43 |
* VanessaE |
looks at celeron55 |
15:44 |
MTDiscord |
<Benrob0329> https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L4050 |
15:46 |
MTDiscord |
<Benrob0329> sees set_lighting() |
15:48 |
MTDiscord |
<Benrob0329> Does this set a temporary light amount, or an actual fixed amount for both day and night respectively? |
15:49 |
VanessaE |
I assume it's temporary, until something comes along and alters the map |
15:53 |
MTDiscord |
<Benrob0329> So, it looks like it lets you set the current natural lighting for an area, but only within an on_generated |
15:55 |
MTDiscord |
<Benrob0329> But wouldn't that just set the param1? If so, why does this function exist and why is it limited to on_generated? |
15:55 |
MTDiscord |
<Benrob0329> You can set the param1 with a different function from anywhere |
15:56 |
MTDiscord |
<Warr1024> Presumably if you use the mapgen vmanip, you can avoid this problem, but some of the things VaE is trying to do in her API might be too annoying to do in VM-land. |
15:57 |
MTDiscord |
<Warr1024> I think the mapgen voxelmanip is probably the one owned by the mg thread, so manipulations done on it are not subject to the same race cond. |
15:57 |
MTDiscord |
<Warr1024> It sounds like we've got the edge cases cleaned up, though, and we're now down to corner cases... |
15:57 |
MTDiscord |
<Warr1024> So, I said, check the 6-neigborhood ... I think we MAY have to do the 26 |
15:57 |
VanessaE |
6 is expensive enough |
15:57 |
VanessaE |
look at the paste |
15:58 |
MTDiscord |
<Warr1024> well, you're actually paying for 12 |
15:58 |
VanessaE |
26 would slow things down so much it would pretty well kill the benefits of spreading-out the load. |
15:58 |
MTDiscord |
<Warr1024> because you're retrieving the nodes redundantly |
15:58 |
MTDiscord |
<Benrob0329> I don't think it'd be a big change, since VMs have equivalents to get and set_node and the whole thing will be loaded. |
15:58 |
MTDiscord |
<Warr1024> also, you're talking about 20 node checks per mapblock; that's not expensive |
15:59 |
MTDiscord |
<Warr1024> ABM neighbor checks do 26 node checks per node, that's expensive. 26 per mapblock is just under 1/1000 as expensive |
15:59 |
MTDiscord |
<Benrob0329> If it'd avoid the problem entirely it might be worth a shot |
15:59 |
MTDiscord |
<Benrob0329> But thats just my opinion :-) |
15:59 |
VanessaE |
Ben: but what happens when all actions have been done on a block and it's time to write it out, but in the meantime the user has already started digging/building in the hot area? bang, bye bye house. |
15:59 |
VanessaE |
(or whatever) |
15:59 |
MTDiscord |
<Warr1024> Plus, we're talking about mapgen cases. You can afford a lot more expense to get mapgen right, because you only mapgen once per block, so it's both a one-time cost, and you also have more impetus to get it right because any mistakes will last a long time |
16:00 |
VanessaE |
well it isn't just once. |
16:00 |
VanessaE |
it's continuous |
16:00 |
VanessaE |
if a block appears to have unloaded neighbors, it has to be moved to the end of the queue |
16:00 |
VanessaE |
which means all the blocks out at the edge of the generated area are gonna be doing that |
16:01 |
MTDiscord |
<Warr1024> for dx = -32, 32, 32 do for dy = -32, 32, 32 do for dz = -32, 32 32 do if minetest.get_node({x = pos.x + dx, y = pos.y + dy, z = pos.z + dz}).name == "ignore" then return end end end end return true |
16:01 |
MTDiscord |
<Benrob0329> Ve: No? If you're doing this in on_generated the whole thing will still be ignore until it finishes |
16:01 |
VanessaE |
checking for not-emerged neighbors over and over and repeatedly being moved to the end of the queue |
16:01 |
VanessaE |
Ben: nope. we're getting "half-emerged" blocks due to chunk overlap[ |
16:01 |
VanessaE |
the engine is passing me blocks that are not safe to write to |
16:02 |
MTDiscord |
<Benrob0329> Thats not what I was referring to |
16:02 |
MTDiscord |
<Warr1024> We are arguing about microseconds for a thing that may or may not actually fix your problem. First find out if it fixes the problem. Then find out if the microseconds you pay for it are worth it. |
16:02 |
VanessaE |
Ben: maybe not but that's what Warr and I'm dealing with and what Warr is trying to offer help on :) |
16:03 |
MTDiscord |
<Warr1024> Sometimes making shit work just comes at a cost, and you have to decide whether making it work right is worth the cost. Maybe there's an optimization you can find later, maybe not. |
16:03 |
VanessaE |
Warr: did you see the paste though? |
16:03 |
MTDiscord |
<Warr1024> For one thing, if you want an easy optimization, just MRU-cache all known-loaded mapblocks and you can do the lookup there instead of hitting the API. |
16:03 |
MTDiscord |
<Warr1024> if you mean vs8E760R then yes, I looked at it |
16:03 |
VanessaE |
ok |
16:03 |
MTDiscord |
<Warr1024> you're checking 12 neighbors (actually 6 neighbors twice each) |
16:04 |
VanessaE |
Warr: yeah. and it DID help |
16:04 |
MTDiscord |
<Warr1024> yeah, you're down from edges to just corners |
16:04 |
MTDiscord |
<Warr1024> that's why I think the 26 neighbors will help |
16:04 |
VanessaE |
ok |
16:04 |
MTDiscord |
<Warr1024> take a look at the code I pasted |
16:04 |
VanessaE |
I saw |
16:04 |
MTDiscord |
<Warr1024> er, wrote I guess |
16:04 |
VanessaE |
just trying to figure out if there's a cleaner way is all |
16:04 |
MTDiscord |
<Warr1024> Unrolling the loops might make it a tad faster on PUC lua, though I expect the different would be immeasurable on JIT. |
16:05 |
MTDiscord |
<Warr1024> if by "cleaner" you mean "more clever" / "optimized" then maybe. If you mean conceptually simpler, probably not. |
16:05 |
VanessaE |
I mean less kludgy :) |
16:06 |
MTDiscord |
<Warr1024> Do you mean less kludgey conceptually or do you mean less ugly code-wise? |
16:06 |
MTDiscord |
<Warr1024> far as I know, nested loops are about the cleanest way to do it starting from zero. |
16:06 |
MTDiscord |
<Warr1024> You could pre-build a table of the 26-neighborhood and walk it once, and that'd be locally cleaner but globally mesiser. |
16:06 |
MTDiscord |
<Warr1024> unless you get some kind of reuse of the 26 neighbors |
16:07 |
MTDiscord |
<Warr1024> I actually also check the center pos because I figured the extra code complexity of checking for that special case was not worth the theoretical perf savings (you incur overhead for the check on every iteration to compensate for 1 fewer API call, so that shrinks the impact) but you can add that in if you want; it COULD be helpful. |
16:08 |
MTDiscord |
<Warr1024> Anyway, find a shitty solution first before you worry about finding a good one. There's no value in a good non-solution, so movement solution-ward is more important than movement good-ward. |
16:09 |
VanessaE |
good point. |
16:09 |
MTDiscord |
<Warr1024> Once you confirm that the 26 neighbor scan works, one thing you can try is trimming it down to just the 8 corners. That should work assuming that mapchunks are always more than 1 mapblock wide, and you already make that assumption very strongly elsewhere in the code. |
16:09 |
sfan5 |
does it really matter if a few mapblocks are missing plants? |
16:10 |
sfan5 |
because personally I'm sure I'd have not bothered with this |
16:10 |
MTDiscord |
<Warr1024> It matters a hell of a lot more than whether we spend 100 microseconds vs 110. |
16:10 |
MTDiscord |
<Warr1024> it's quite noticeable when the plants are dense. |
16:10 |
MTDiscord |
<Warr1024> If people weren't already bothered by it then it'dn't've been reported in the first place. |
16:11 |
VanessaE |
sfan5: it matters because it means either the engine is broken or my code is, though in this case it's the engine. it should *not* be returning blocks (==triggering on_generated()) for areas that have overlap until after the overlap has been satisfied. |
16:11 |
MTDiscord |
<Warr1024> I wouldn't go so far as to call this a "bug" in the engine; it's more like UB tbh |
16:12 |
sfan5 |
I'm not saying it's not a bug, just question the tradeoff of unfixed end result <-> time spent on workarounds |
16:12 |
sfan5 |
questioning* |
16:12 |
VanessaE |
it's not just one mapblock here or there though |
16:12 |
MTDiscord |
<Warr1024> I think MT has a few things like that, where behavior at boundary cases is just sort of not guaranteed by the engine, and any assumptions we make about it behaving the way we think it should aren't actually reliable... |
16:12 |
VanessaE |
https://github.com/minetest/minetest/issues/11134 |
16:12 |
VanessaE |
take a look at the first screenshot |
16:12 |
VanessaE |
this is what happens without any workarounds. |
16:13 |
MTDiscord |
<Warr1024> Anyway, for the 8 corner checks, you can just make the step size of my loops 64 instead of 32 and that will hit the 8 corners already. Since it's only 8, it'd also be feasible to hand-unroll if that's your thing. |
16:14 |
MTDiscord |
<Warr1024> Frankly it's easy to justify spending a ton of effort fixing minor problems when you consider how much time this community in general pours into fixing non-problems. |
16:14 |
VanessaE |
I wonder... does the overlap extend vertically? |
16:14 |
VanessaE |
or could I just check the 8 X/Z neighbors? |
16:14 |
MTDiscord |
<Warr1024> If you mean "does MT treat some dimensions specially so that I don't need to worry about the Y axis" then I'd say no, don't rely on that assumption. |
16:15 |
MTDiscord |
<Warr1024> If something can happen on X or Z then assume it can happen on all 3 axes. |
16:15 |
VanessaE |
no, I mean the mud flow feature that originally gave rise to this overlap thing |
16:16 |
MTDiscord |
<Warr1024> Whether the feature has an effect on the Y axis, I'm pretty sure the underlying mechanisms don't prevent overlap on the Y axis. Trees can other schematics/decorations can also cross the Y boundaries. |
16:16 |
VanessaE |
btw, checking all 26 (well 27) does appear to completely eliminate the empty blocks. |
16:16 |
MTDiscord |
<Warr1024> Mudflow predates current mapgens, but current mapgens offer some features that almost certainly rely on the behavior, regardless of whether the reason it was originally created for is even valid or not anymore. |
16:17 |
VanessaE |
https://pastebin.com/macWZYAn there's the working code |
16:17 |
MTDiscord |
<Warr1024> Try the 8 corners. Bump the loop step from 32 to 64. If that works too, that's already a big optimization and gets you back close to where you were with 6 (only actually working now) |
16:17 |
VanessaE |
(line 427) |
16:17 |
MTDiscord |
<Warr1024> If 8 doesn't work though then I posit that even the 26 is perfectly livable. |
16:18 |
MTDiscord |
<Warr1024> If you start measuring lagspikes with this code that you didn't see before, then we can revisit it. |
16:18 |
MTDiscord |
<Warr1024> Also, try either (1) using get_node, or (2) storing get_node_or_nil in a local, so you only have to call the API once. |
16:19 |
MTDiscord |
<Warr1024> Lua's official optimization guide suggests that often recalculating something can be more efficient than storing it, but I think for something like an actual function call, esp. an API call, it's probably better to store it in a local... |
16:20 |
MTDiscord |
<Warr1024> I dunno if get_node vs. get_node_or_nil has any meaningful performance difference, but heuristically I'd expect them to be close enough to the same that it's very unlikely to matter, and get_node already collapses different kinds of ignores into a single check. |
16:22 |
MTDiscord |
<Warr1024> https://pastebin.com/ttKkSPKi is the "call get_node_or_nil only once" route I'm thinking |
16:22 |
VanessaE |
yeah, I'm on it |
16:23 |
MTDiscord |
<Warr1024> If it still works, we can see if the unrolling route is better ... I guess whether it's "cleaner" or not depends a bit on the overall style you're going for in the code... |
16:24 |
MTDiscord |
<Warr1024> I tend to prefer that code is closer to its conceptual form and has as little baked-in optimization as tolerable, but there are places where it makes enough difference that that ideal would come at too high a cost. |
16:25 |
VanessaE |
8 corners (plus center) seems to work perfectly |
16:25 |
VanessaE |
https://pastebin.com/6wC4WZe0 |
16:25 |
MTDiscord |
<Warr1024> This sort of check only really works for detecting "mapgen done" cases in specific cases where mapgen is done contiguously as a function of player exploration; if you just try to manually emerge stuff or use force-loads, probably it could result in some areas that stay queued forever, until a player explores the surrounding areas. |
16:27 |
MTDiscord |
<Warr1024> It looks good to me. I think assuming you don't discover any new problems in your testing, it's release-worthy, and would be good to get out to anyone using it ASAP so at least the known issue is dealt with. |
16:27 |
VanessaE |
*nod* |
16:28 |
MTDiscord |
<Warr1024> It's possible it could be better optimized but I kinda doubt that optimizations could have enough of an impact to make it worth holding up a release, or even making a hotfix release; probably better to just worry about optimizations if you do an "optimization pass" on it later. |
16:28 |
MTDiscord |
<Warr1024> Like, I mean, across the whole mod, using the profiler and such... |
16:28 |
VanessaE |
there's one more optimization I need to add though - a flag to pause processing the block log if a mapblock comes up in it that it's seen before but had moved to the end, until the user triggers generation of more new terrain |
16:29 |
VanessaE |
which would stop it from continuously churning when the only things left in the log are the blocks out at the edges of the terrain |
16:30 |
MTDiscord |
<Warr1024> Ah, what I do for that sort of thing is usually move the queue into a "batch" variable, recreate the queue as an empty list, and then process through the batch, so that things can be added to the queue without affecting the batch; this also allows me to avoid having to use the possibly-expensive table.remove call, since I can just walk the frozen batch array by index. |
16:31 |
MTDiscord |
<Warr1024> Normally I do this for stuff where the batch doesn't live past the end of a function call (i.e. I don't have to store it outside of function-locals) but you could easily store the batch somewhere and just make sure you end the current-tick processing when the batch is empty, i.e. only capture a batch of work at the very beginning of a tick if the current batch is empty. |
16:31 |
MTDiscord |
<Warr1024> Depending on how you handle it, it's sort of a wash anyway though. |
16:31 |
MTDiscord |
<Warr1024> Like, there are a bunch of ways you can do this which are probably all about equivalent. |
16:32 |
MTDiscord |
<Warr1024> If you have only one block in the queue and you try to do 100 blocks per tick, and you end up just processing that 1 block 100 times, yeah, that'd feel really silly :-) |
16:32 |
VanessaE |
heh |
16:32 |
VanessaE |
it's not 100 blocks per tick tho |
16:32 |
VanessaE |
it's 100 *actions* per tick |
16:32 |
VanessaE |
or whatever number |
16:33 |
VanessaE |
(which under Dreambuilder, with all the flora its mods add, would be roughly 2 complete mapblocks per tick, as the action list is 50-some items long) |
16:37 |
VanessaE |
another I'd like to do, but it would be kinda expensive, would be to periodically [re-]sort the map block log (say once every 5s), to put map blocks closest to the player(s) early in the list, so that they populate first before the more distant ones |
16:40 |
MTDiscord |
<Warr1024> Interesting; that implies that as you add more and more mods that depend on biome_lib, the farther behind it's possible to make the processing queue become... |
16:40 |
VanessaE |
yes |
16:40 |
VanessaE |
but you can compensate for that by increasing the actions per tick setting |
16:40 |
VanessaE |
up to the limit of your CPU anwyay |
16:41 |
VanessaE |
my old PC here can churn through up to around 500 actions per tick in singleplayer |
16:41 |
VanessaE |
I figure each person/admin should set that variable to whatever works best for their hardware and mod collection |
16:50 |
MTDiscord |
<Warr1024> Sure, it can be fine-tuned. I just think that if you don't explicitly tune a system, it should have sane defaults, and if "sane" is dependent on circumstances, then the defaults ideally should be too. |
16:52 |
VanessaE |
I aimed for that, actually. the default of 100 actions/tick is about the speed it used to be when the dtime stuff was all screwed up until a couple days ago |
16:52 |
VanessaE |
(it was a mess) |
16:52 |
MTDiscord |
<Warr1024> Yeah, finding good adaptive optimum defaults is sometimes pretty hard. It's just beautiful when you can make a system work that way. |
16:52 |
VanessaE |
though since to the user it feels much faster than before, when it used to process an entire chunk at a time |
16:54 |
MTDiscord |
<Warr1024> I found one little microoptimization to walking a vmanip that helped me out a ton on cutting down mapgen latency. Much of the time I'm stuck doing chunk-at-a-time because I'm doing everything inside on_generated. |
16:54 |
* VanessaE |
ponders how to implement the pause/resume idea |
16:54 |
MTDiscord |
<Warr1024> There's that "area" thing that lets you calculate offset within the content data based on x/y/z coords. The trick is that I do that only once per linear "walk" through the volume |
16:54 |
VanessaE |
if chunk-at-a-time is a prob, maybe you could do what I did and split the chunk into blocks? |
16:55 |
MTDiscord |
<Warr1024> If I did that then I'd have to suffer through all the same half-loaded-ness and asynchrony shit you've had to put up with, and I'm not sure if the cost is worth it to me. |
16:55 |
VanessaE |
haha no |
16:55 |
MTDiscord |
<Warr1024> It's a very small piece of all I have to maintain, and tbh I don't think mapgenning happens often enough to bother me. |
16:56 |
MTDiscord |
<Warr1024> If I added a lot of biome diversity to my game, then that could happen, but I sorta don't like the idea of categorical biomes anyway. |
16:56 |
VanessaE |
the half-loaded-ness was happening before, in chunk-at-a-time mode too |
16:56 |
VanessaE |
ok that's our official term for MT mapgen. half-loaded-ness :D |
16:57 |
MTDiscord |
<Warr1024> half-assed-ness might be better :-) |
16:57 |
VanessaE |
lol |
16:57 |
MTDiscord |
<Warr1024> esp. since it's not really quite "half" |
16:57 |
MTDiscord |
<Warr1024> It feels more like "schematic spillover but no terrain" is like 1/3 and "terrain but no schematic spillover" is like 2/3... |
16:57 |
MTDiscord |
<Warr1024> or "mud flow" maybe, though I'm not sure if that's still a thing...? |
16:58 |
MTDiscord |
<Warr1024> but schematic sure as hell are... |
16:59 |
MTDiscord |
<Warr1024> By far the biggest cost I pay in mapgen is when I have to walk the entire vmanip and do manipulations on every node in it, especially when I have to do neighbor checks. In NodeCore I have a rule that Lode Ore is not allowed to be exposed to air, so I have to detect and convert these, and that includes a neighbor check against every matching node in the chunk. That's probably peak expensive |
16:59 |
MTDiscord |
<Warr1024> It can be done in at least reasonable time though, at least. I'm not sure if breaking it down would be worth it anyway. |
17:00 |
MTDiscord |
<Warr1024> Often you have to make latency/throughput trade-offs, and doing it all in the mapgen callback optimizes throughput at the cost of latency. Sure, you may get lagspikes while exploring, but you can maintain a higher average movement rate that way. |
17:02 |
MTDiscord |
<Warr1024> Performance/scalability trade-offs seem to be one of the more counterintuitive trade-offs for a lot of devs, apparently; I've seen a few 10+-year professional engineers who were puzzled by the distinction. It might be a thing you'll have to consider though when you think about what kind of "optimized" you're shooting for. |
17:03 |
VanessaE |
well the way my code is written, you could set the queue run ratio to like, -6000 or so, and you'd get the same effect -- everything for a given mapblock would get done in one big glob before the next one is even recorded into the log |
17:05 |
VanessaE |
(that's roughly a whole chunk's worth of content in DB) |
17:15 |
VanessaE |
Warr: it appears to be sufficient to do check if minetest.get_node(pos).name == "ignore", without the nil check |
17:16 |
VanessaE |
so now we know, "ignore" is half-loaded |
17:19 |
VanessaE |
but nil can be half-loaded too. |
17:20 |
MTDiscord |
<Warr1024> It is a little bit humorous how far you have to check around a thing for at-least-partial-loadedness before you can determine whether a thing is itself almost-certainly-fully-loaded or not yet... |
17:21 |
VanessaE |
heh |
17:22 |
MTDiscord |
<Warr1024> and yeah, we're at the "almost certainly" standard, not absolutely certain, because you can never really be absolutely certain when you can only really verify empirically but not exhaustively, and that you're in that situation in the first place because of sorta-nonsensical behavior or a general lack of guarantees... |
17:23 |
MTDiscord |
<Warr1024> We have to test 2 mapblocks out because currently we rely on finding a definitely not initialized at all mapblock for the ignore check to always work. |
17:31 |
VanessaE |
Warr: I added a comment to https://github.com/minetest/minetest/issues/11134 , care to chime in? |
17:34 |
MTDiscord |
<Warr1024> I guess the core issue to me is the fact that parts of mapgen are happening in a background thread that may take a snapshot of map data (to be modified by the mapgen thread) and then write those mapblocks back with modifications based on that snapshot, while apparently some of the same mapblocks may have been made available to the lua state for things like set_node stuff in the meantime, and thus changes made in the main game thread |
17:34 |
MTDiscord |
are not honored by the mapgen thread. |
17:35 |
MTDiscord |
<Warr1024> It could be possible for them to avoid this problem by just merging the mapblocks (e.g. read from the game-state one wherever the mapgen one has ignores) during the "apply changes from mg thread" step. It could be a bit expensive, maybe (if they're doing a zero-copy reference replacement on the mapblocks) but it might be necessary. It could be possible to track a dirty-flag on both sides or something to determine if this actually |
17:35 |
MTDiscord |
needs to be done so we don't have to pay the price in cases where it's not relevant. |
17:36 |
VanessaE |
I meant add your remarks to the issue :) |
17:36 |
MTDiscord |
<Warr1024> A lot of MTE's problem is the way they give you a Phillips screwdriver, but they only test it on steel screws, and as soon as you try using it on a brass screw, they tell you to see if you can reproduce the problem on a steel one instead. |
17:37 |
MTDiscord |
<Warr1024> Eh, I guess I could add those remarks... |
17:37 |
VanessaE |
haha |
17:37 |
MTDiscord |
<Warr1024> Not sure if they're well-enough informed. |
17:38 |
MTDiscord |
<Warr1024> I guess I sorta just don't want to have to try digging into the engine code itself and verify what's going on. It's just what my intuition is telling me is happening, and at least what we've observed is consistent with that intuitive understanding, but it's not necessarily exclusive. |
17:40 |
VanessaE |
only question now is, since "not n or n.name == "ignore" means a half-emerged block, how do I check if a block is really, truly, honest-to-G*d not emerged at all |
17:40 |
VanessaE |
(so that mapgen can finish out to the edges of the player's view and the block log can run dry) |
17:42 |
MTDiscord |
<Warr1024> I suspect you can't |
17:43 |
VanessaE |
didn't think so. |
17:43 |
MTDiscord |
<Warr1024> Your queue may in fact never actually run dry |
17:43 |
MTDiscord |
<Warr1024> There might be ways you could optimize away having to run the queue every damn cycle, like doing some kind of partitioning with an "are there at least players reasonably close to this" check or something. |
17:44 |
VanessaE |
yeah |
17:44 |
MTDiscord |
<Warr1024> Also, in the extremal case where you've explored literally the entire world, you'd have to add explicit special checks for mapgen_limit |
17:44 |
VanessaE |
that gets back to what I was musing about earlier |
17:44 |
MTDiscord |
<Warr1024> except that as far as I can tell, the actual math for mapgen_limit is intractable ? |
17:44 |
MTDiscord |
<Warr1024> also, mapgen_limit can be raised later, so you don't necessarily want to let those mapblocks leave the queue either... |
17:45 |
VanessaE |
if I could somehow sort the block log every now and then, putting them in order by distance to any player |
17:45 |
MTDiscord |
<Warr1024> I guess the biggest issue to worry about is that your queue will grow in proportion to the surface area of the explored region, so it could become quite large eventually |
17:46 |
VanessaE |
nah |
17:46 |
VanessaE |
not a big worry there |
17:46 |
VanessaE |
it only needs a couple dozen bytes per mapblock |
17:46 |
MTDiscord |
<Warr1024> eh, well, it worries me a bit, especially since you have to manage persisting this massive queue. |
17:47 |
MTDiscord |
<Warr1024> Assuming you explore the entire world uniformly, you can have up to 100M mapblocks in the queue, representing the very edges of the map. Those would actually never get fully biome_lib_bed. |
17:47 |
VanessaE |
true |
17:48 |
MTDiscord |
<Warr1024> You could create a pathological case that'd probably have orders of magnitude more, but that's maybe not worth worrying about realistically. |
17:48 |
VanessaE |
but that'd still be only a couple of gigs, and it would take weeks to explore the world to that degree |
17:49 |
|
MTDiscord1 joined #minetest-hub |
17:49 |
MTDiscord1 |
<Warr1024> A couple of GB is kinda a lot to potentially be writing back to mod_storage or whatever every 5.3 seconds... |
17:50 |
MTDiscord |
<Warr1024> tbh it feels like maybe an inside-out approach would make more sense, like having on_generated replace one node within the mapblock with a special node that indicates "initialization not done" so that the state of mapblocks is guaranteed durable, then having logic to scan and find these and do the initialization as needed. |
17:51 |
MTDiscord |
<Warr1024> I wonder if node meta is accessible during mapgen? I've really never tried that, and I'd be worried about its threadsafeness. |
17:51 |
VanessaE |
oh that's a funky one |
17:52 |
MTDiscord |
<Warr1024> Also it's a pain to try to replace a node in a mapblock with another node because you'd want to be able to replace the original node afterwards, and there's no good way to store the original node in a way you can definitely recover it without metadata, I think. |
17:53 |
VanessaE |
if you write to the map FAST enough, say placing a grid of fenceposts or torches or something else thin and easy to see past, you'll get corrupted data under them, but only in the nodes immediately under -- i.e. the corruption will follow the same grid pattern, while at a larger scale following the pattern of those formerly-bare strips of terrain we just worked around |
17:53 |
MTDiscord |
<Warr1024> Basically I'm trying to think if there's a way to do this ACID-transactionally under the assumption that the underlying thing mechanism always either saves or doesn't save a set of mapblock changes. |
17:53 |
VanessaE |
like, grass will inexplicably turn to stone or gravel, under the fence/torch/etc |
17:54 |
MTDiscord |
<Warr1024> Wait, what, is this another different bug? |
17:54 |
VanessaE |
it's some oddity I ran into a few days ago while banging my head against the bare mapblocks issue we just solved |
17:54 |
VanessaE |
I just chalked it up to MT being stupid ;) |
17:55 |
MTDiscord |
<Warr1024> Something that happens on the fringe, or can happen anywhere? |
17:55 |
VanessaE |
at the time, I didn't see a pattern, but after today's discussion, it was most definitely on chunk edges |
17:56 |
VanessaE |
and you'd see complaints in the console about node meta not being found or some such |
17:56 |
MTDiscord |
<Warr1024> huh, neighbor data corruption is ... not something I have a theory about, even... |
17:56 |
MTDiscord |
<Warr1024> I've seen a lot of console complaints about shit failing to load, like node timers not deserializing right... |
17:57 |
VanessaE |
yeah, this was nothing important. I might dink around later and see if I can reproduce it just to get you a screenshot |
17:57 |
MTDiscord |
<Warr1024> That's one of the reasons why nearly every node timer I use needs to have a "patrol" ABM that makes sure it can't be lost. I expect my worlds to "self-heal" and never allow themselves to become stuck indefinitely in a state where they're not applying the behavior they're supposed to be. |
18:07 |
sfan5 |
VanessaE: you don't have num_emerge_threads > 1 do you? |
18:08 |
VanessaE |
nope |
18:08 |
sfan5 |
good to know |
18:08 |
VanessaE |
it's not set at all, so default 1 |
18:08 |
VanessaE |
wouldn't do much good anyway since Lua hooks into it |
18:09 |
sfan5 |
true |
18:09 |
MTDiscord |
<Warr1024> I assume you can't set it to 0 and force synchronous emerges. That'd "solve" your mapgen problem, probably, at the expense of causing others... |
18:10 |
MTDiscord |
<Warr1024> Though if you have a 4-core server that's running 4 relatively steadily-busy worlds on it, then the multithreading stuff might not be as valuable to you as you might think... |
18:11 |
MTDiscord |
<Warr1024> If I'm looking for the basics, like just some tall grass / flowers / trees, i.e. use-cases that the standard decoration API already offers, is there any advantage to using biome_lib for it instead? Or was biome_lib created before that stuff was available standard? |
18:13 |
MTDiscord |
<Warr1024> Heh, I guess I HOPE there's no reason I should want to use it in that case anyway, since it looks like it'd be license-incompatible with pretty much everything I release anyway. |
18:31 |
VanessaE |
it was created long before decorations existed, but imho its API is more flexible. |
18:41 |
Jordach |
nothing like an afternoon of porting code |
18:54 |
|
Peppy joined #minetest-hub |
19:39 |
|
hlqkj_ joined #minetest-hub |
19:40 |
|
hlqkj joined #minetest-hub |
20:10 |
VanessaE |
Warr1024: https://gitlab.com/VanessaE/biome_lib/-/commit/e346fd599f3877a89618c7b811fea3ae6d7e0117 |
20:17 |
VanessaE |
I still feel like handling of the blocks that can't yet be populated is a little.. kludgy (particularly when it comes to re-adding them to the main list to try hitting them again) but this is 10x better than before :) |
20:17 |
VanessaE |
and I'm wiped-out. |
20:17 |
VanessaE |
I/O error reading /dev/brain: device not ready |
20:20 |
Krock |
attempting to unmount /dev/brain |
20:46 |
|
Peppy joined #minetest-hub |
20:47 |
|
homthack joined #minetest-hub |
22:08 |
|
BillyS joined #minetest-hub |
22:47 |
|
DS-minetest joined #minetest-hub |