Time |
Nick |
Message |
01:01 |
|
ANAND joined #minetest-dev |
01:26 |
|
erlehmann joined #minetest-dev |
01:33 |
|
nephele joined #minetest-dev |
01:39 |
|
Miner_48er joined #minetest-dev |
02:16 |
|
erlehmann joined #minetest-dev |
04:02 |
|
Taoki joined #minetest-dev |
04:09 |
|
erlehmann joined #minetest-dev |
04:32 |
|
fluxflux joined #minetest-dev |
04:45 |
|
calcul0n joined #minetest-dev |
07:06 |
|
nerzhul joined #minetest-dev |
07:14 |
|
mizux joined #minetest-dev |
08:00 |
|
ShadowNinja joined #minetest-dev |
08:03 |
|
absurb joined #minetest-dev |
08:08 |
|
ShadowBot` joined #minetest-dev |
08:11 |
|
sofar_ joined #minetest-dev |
08:16 |
|
Foz joined #minetest-dev |
08:53 |
ANAND |
If I want to use MT's builtin profiler, would it suffice for me to call g_profiler->avg with the value? |
08:54 |
ANAND |
Also, what's the recommended way to time segments of code? I see both TimeTaker (src/util/timetaker.h) and porting::getTimeMs being used across the code-base. |
08:58 |
ANAND |
Ok, TimeTaker uses porting::getTime internally, nvm :) |
09:49 |
|
Thomas-S joined #minetest-dev |
09:49 |
|
Thomas-S joined #minetest-dev |
10:46 |
|
NetherEran joined #minetest-dev |
10:48 |
|
pmp-p joined #minetest-dev |
11:09 |
|
Fixer joined #minetest-dev |
12:12 |
|
Beton joined #minetest-dev |
12:46 |
|
fluxflux joined #minetest-dev |
13:32 |
|
lisac_ joined #minetest-dev |
14:20 |
|
H4mlet joined #minetest-dev |
14:24 |
ANAND |
#9851 |
14:24 |
ShadowBot |
https://github.com/minetest/minetest/issues/9851 -- Improve performance of Camera::update by ClobberXD |
14:29 |
sfan5 |
did you profil the timings of that function by chance? |
14:30 |
sfan5 |
profile* |
14:31 |
ANAND |
sfan5: I added auto start = porting::getTimeMs at the start of Camera::update |
14:32 |
ANAND |
And added g_profiler->avg("Camera::update", (float) porting::getTimeMs() - start) at the end of the function |
14:32 |
sfan5 |
i believe there is a scope profiler class for that |
14:32 |
ANAND |
True |
14:33 |
ANAND |
I used the same code to profile only the changed segment in the function instead of the whole function |
14:33 |
ANAND |
Weirdly, the measured time was the same as the time taken by the entire function |
14:34 |
ANAND |
i.e. around 0.06ms to 0.07ms |
14:34 |
ANAND |
I did recompile after making the change, btw :) |
14:42 |
|
H4mlet_ joined #minetest-dev |
14:42 |
|
erlehmann joined #minetest-dev |
14:46 |
|
Lone_Wolf joined #minetest-dev |
14:47 |
ANAND |
I noticed something else: Many profiler entries' values kept flickering in and out of existence, and didn't remain "still", for a lack of a better word |
14:47 |
ANAND |
The value of the profiler entry I added for testing this PR was invisible around 95% of the time. |
14:48 |
ANAND |
Is this intended? |
14:48 |
|
Fixer_ joined #minetest-dev |
14:51 |
sfan5 |
doesn't that mean Camera::update() isn't called "95% of the time"? |
14:53 |
|
proller joined #minetest-dev |
14:56 |
|
Fixer joined #minetest-dev |
14:57 |
ANAND |
Uh no, not really. Camera::update is called every frame. |
14:58 |
|
Fixer joined #minetest-dev |
14:58 |
sfan5 |
¯\_(ツ)_/¯ then |
15:00 |
ANAND |
I tried to time this by printing the duration to stdout |
15:01 |
ANAND |
My computer slowed down to a leisurely-walk - probably because of flushing stdout every frame(?) |
15:04 |
ANAND |
I wonder if there's a light-weight way to get the average time that doesn't involve the profiler |
15:04 |
ANAND |
I suppose I could do it manually in C++... |
15:51 |
|
Wuzzy joined #minetest-dev |
15:51 |
Wuzzy |
So what is special about `leveled` and falling nodes? |
15:52 |
Wuzzy |
In the code I see that `leveled` has a special branch, but its a very weird if condition |
15:53 |
|
erlehmann joined #minetest-dev |
15:53 |
sfan5 |
it would suffice to say that leveled nodes have their level added if the same leveled node falls on it |
15:53 |
sfan5 |
roughly |
15:53 |
Wuzzy |
if bcn and (not bcd or bcd.walkable or (core.get_item_group(self.node.name, "float") ~= 0 and bcd.liquidtype ~= "none")) then if bcd and bcd.leveled and bcn.name == self.node.name then |
15:54 |
Wuzzy |
it looks like the `leveled` branch is not triggered if the float group does not exist. this looks like a bug |
15:55 |
sfan5 |
are you looking at falling.lua from 5.2? |
15:57 |
Wuzzy |
no |
15:57 |
|
ANAND_ joined #minetest-dev |
15:58 |
Wuzzy |
oh wait |
15:58 |
sfan5 |
not sure where you're seeing that exact condition then |
15:58 |
Wuzzy |
haha. |
15:58 |
sfan5 |
https://github.com/minetest/minetest/blob/master/builtin/game/falling.lua#L198-L208 |
15:59 |
Wuzzy |
doesnt work apparently |
15:59 |
Wuzzy |
when i drop a leveled node into another one, the falling node just drops as an item |
15:59 |
* Wuzzy |
tries buildable_to |
16:00 |
sfan5 |
did you set "leveled = true" (or maybe a number?) in the nodedef (not the groups)? |
16:00 |
sfan5 |
I also have no idea where self.node.level is supposed to come from |
16:00 |
sfan5 |
(I did not test this either) |
16:01 |
Wuzzy |
ohhhhhhh |
16:01 |
Wuzzy |
this is probably a bug then |
16:01 |
Wuzzy |
there is no node.level |
16:01 |
Wuzzy |
afaik node table only have node.name, node.param2 and node.param1 |
16:01 |
sfan5 |
well it has a fallback for self.node.level |
16:01 |
sfan5 |
so try leveled = 1 in the nodedef |
16:03 |
Wuzzy |
def.leveled is supposed to be a number tho |
16:03 |
Wuzzy |
its the default node level |
16:04 |
Wuzzy |
so setting leveled = true in node def would be wrong, acording do lua_api.txt |
16:04 |
Wuzzy |
it looks like this code is just completely wrong, it interprets lua_api wrong |
16:04 |
Wuzzy |
i.e.: node.level, nodedef.leveled == true |
16:05 |
sfan5 |
¯\_(ツ)_/¯ |
16:05 |
Wuzzy |
its no surprise it doesnt work ? |
16:05 |
sfan5 |
so does leveled = 1 work? |
16:06 |
Wuzzy |
I am strongly suspecting this leveled code has never been tested ? |
16:07 |
Wuzzy |
no no the code is just wrong, it needs to be written. pointless to tweak my test node definition |
16:10 |
sfan5 |
I know that it is wrong but I am still wondering if leveled = 1 works |
16:17 |
Wuzzy |
nope |
16:19 |
Wuzzy |
okay i managed to change the code to make leveled actually work |
16:19 |
Wuzzy |
but i have discovered another bug with this ? |
16:19 |
Wuzzy |
minetest.add_node_level apparently has no bounds check |
16:20 |
Wuzzy |
so if a high-level node drops into another high-level node, the level will just wrap around... ? |
16:21 |
|
nerzhul_ joined #minetest-dev |
16:25 |
Krock |
Wuzzy: definitely a feature to ease.. uuhh.. something |
16:28 |
Wuzzy |
heh |
16:42 |
|
proller joined #minetest-dev |
16:50 |
|
longerstaff13 joined #minetest-dev |
16:54 |
Krock |
Wuzzy: https://github.com/minetest/minetest/pull/9462/files#r423180314 |
16:55 |
Wuzzy |
idk. maybe due to dev request? |
16:56 |
Wuzzy |
i dont remember everything. this pr is no >2 months old :/ |
16:56 |
Wuzzy |
maybe scroll through the resolved conversations (i cant currently read them myself) |
16:57 |
Krock |
checking |
16:58 |
Krock |
no such conversation |
16:58 |
Wuzzy |
WTF |
16:58 |
Krock |
actually |
16:58 |
Krock |
https://github.com/minetest/minetest/pull/9462#discussion_r388524713 <-- sfan5 |
16:58 |
Krock |
SerializationError is thrown of i/ostream |
16:58 |
Krock |
PacketError is thrown for NetworkPacket |
16:59 |
Krock |
hence this may not be changed |
16:59 |
Wuzzy |
waiting for sfan5 then ? |
16:59 |
Krock |
fixing this now |
17:13 |
Wuzzy |
ok thx |
17:22 |
sfan5 |
that means half of the code wasn't actually backwards-compatible then |
17:25 |
Wuzzy |
wait what? |
17:26 |
Wuzzy |
oh. iirc i made that claim before the change requests came flooding in. but its now >2 months ago i dont remember everyhing :/ |
17:26 |
nerzhul_ |
guyz |
17:26 |
nerzhul_ |
i want to be honest |
17:26 |
nerzhul_ |
but except if we import irrlicht in minetest source tree, i don't see how we can make minetest rendering modern |
17:26 |
nerzhul_ |
irrlicht doesn't expose the video driver creation methods, very nice if you want to implement your own IrrlichtDevice which needs it haha |
17:28 |
nerzhul_ |
i'm trying to implement wayland windowing, but i'm stuck becausei cannot create video drivers outside of irrlicht itself |
17:32 |
sfan5 |
I suggest implementing that inside irrlicht first and then seeing if it can be moved outside of it |
17:33 |
Wuzzy |
http://irrlicht.sourceforge.net/forum/viewtopic.php?f=2&t=52615 |
17:33 |
nerzhul_ |
oh i just use the interface but i'm unable to access inside things |
17:42 |
Wuzzy |
sfan5: oh i see it now. its not backwards compatible thanks to the addition of text2... right? |
17:42 |
Wuzzy |
(among other things, i guess) |
17:43 |
sfan5 |
no what I mean is that the old code for world_pos and size doesn't work if SerializationError is never thrown |
17:46 |
Wuzzy |
"old code" = code before this PR? |
17:47 |
sfan5 |
yes |
17:48 |
|
Lone_Wolf joined #minetest-dev |
17:55 |
nerzhul_ |
sfan5: just look at this constructor, which is the underlying layer of the windowing system: https://github.com/TesteurManiak/tek2/blob/5f5f946300f192a78d93a5817dc5bdd3ff17ea1d/cpp_indie_studio/irrlicht-1.8.4/source/Irrlicht/CIrrDeviceStub.cpp#L20 |
17:55 |
nerzhul_ |
and now you know it's lost :D |
17:55 |
nerzhul_ |
timer,, logger, and more things :D |
17:56 |
sfan5 |
I'd say pretty normal for an engine to have those, which irrlicht is |
17:56 |
nerzhul_ |
yep but exposing this class as interface permits to create your own device |
17:56 |
sfan5 |
I just discovered emergequeue_limit_diskonly and emergequeue_limit_generate are per-player, but the documentation makes absolutely not mention of that |
17:56 |
nerzhul_ |
but it's not exposed |
18:00 |
nerzhul_ |
maybe i can cheat the system in another way with composition |
18:00 |
sfan5 |
it should suffice to declare CIrrDeviceStub to have a public constructor in a header file, then you can subclass it |
18:01 |
sfan5 |
maybe you need the attributes too |
18:01 |
sfan5 |
or maybe that doesn't work at all |
18:01 |
nerzhul_ |
all Cxxx class are not exposed by the lib |
18:01 |
nerzhul_ |
and each C class works with multipler other Cxxx class which are hidden too |
18:01 |
nerzhul_ |
i will try another approach, not sure it will work |
18:13 |
sfan5 |
oh god why is all code so broken |
18:17 |
sfan5 |
>Absolute limit of emerge queues |
18:17 |
sfan5 |
who wrote this?? |
18:20 |
rubenwardy |
hmmmm I think? |
18:20 |
sfan5 |
ok that quote doesn't quite show how wrong it is |
18:20 |
sfan5 |
"Limit of emerge queues on disk" |
18:21 |
nerzhul_ |
not me i'm pretty sure, i already saw this code but never understood why we do that |
18:21 |
sfan5 |
there are neither multiple emerge queues nor are the on disk, this settings specifies the amount of emerges *inside* queued *from* disk |
18:21 |
sfan5 |
they* |
18:22 |
sfan5 |
(this is just regarding the settingstypes.txt description, the code itself is mostly fine |
18:28 |
nerzhul_ |
ah but why... createWindow is inside the Linux device constructor... yeah... |
18:28 |
nerzhul_ |
i will not create & destroy a windows just to overwrite logic... pfff |
18:39 |
|
longerstaff13 joined #minetest-dev |
18:42 |
|
LoneWolfHT joined #minetest-dev |
18:58 |
|
kaeza joined #minetest-dev |
19:08 |
sfan5 |
Wuzzy: https://a.uguu.se/vqHaZ0JgkXZK_.png |
19:09 |
Wuzzy |
can't reproduce |
19:09 |
sfan5 |
since you were looking at the wrong falling code earlier maybe you're also running the wrong version? |
19:10 |
Wuzzy |
oh wait |
19:10 |
Wuzzy |
collide_with_objects changes dynamically, right? |
19:10 |
Wuzzy |
dammmmnnn |
19:10 |
sfan5 |
no |
19:11 |
sfan5 |
there is some hacky code that makes it still fall through players but collide_with_objects is never false |
19:11 |
Wuzzy |
weeeeird. ok |
19:11 |
Wuzzy |
i just verified that i was wrong all along... ? |
19:11 |
Wuzzy |
i managed to repro it in devtest, finally |
19:12 |
Wuzzy |
Related: #9852 |
19:12 |
ShadowBot |
https://github.com/minetest/minetest/issues/9852 -- Fix falling nodes not adding to leveled nodes correctly by Wuzzy2 |
19:12 |
Wuzzy |
Unless you want to remove all special handling for leveled nodes in falling node code |
19:13 |
Wuzzy |
and yes, the leveled node handling in falling nodes is currently broken |
19:13 |
sfan5 |
I don't |
19:13 |
Wuzzy |
In my testing, it felt very odd when nodes grew above 64 tho |
19:14 |
Wuzzy |
64 = 1 node height, but max is 127 |
19:14 |
Wuzzy |
what u think? should i allow nodes growing above 64 with falling? |
19:14 |
Wuzzy |
e.g. the resting node has level 50 and the node that falls into it has also level 50. what should be the final level be? |
19:14 |
sfan5 |
shouldn't the nodedef have a max level? |
19:15 |
Wuzzy |
there's none so far ? |
19:15 |
sfan5 |
also doing the calculation in LEVELED_MAX is if you call addLevel for a liquid |
19:16 |
sfan5 |
is potentially wrong* |
19:16 |
Wuzzy |
oh noes. i forgot liquids ? |
19:16 |
sfan5 |
maybe just fix the falling code for now and leave the rest |
19:16 |
sfan5 |
because it sounds like leveled nodes need an entire reword |
19:16 |
sfan5 |
rework* |
19:17 |
Wuzzy |
thats the thin |
19:17 |
Wuzzy |
g |
19:17 |
Wuzzy |
i cant fix falling code without also touching the lua api function |
19:17 |
Wuzzy |
unless i ignore this function in my code and roll my own code. which is kinda silly |
19:17 |
sfan5 |
yes you can |
19:18 |
sfan5 |
if you pretend add_node_level is not buggy |
19:18 |
Wuzzy |
omg |
19:18 |
Wuzzy |
so you want me to do only a half-baked bugfix? boo! |
19:19 |
Wuzzy |
wait. what makes you think minetest.add_node_level is used for liquids? |
19:19 |
sfan5 |
I mean |
19:19 |
Wuzzy |
docs dont mention liquids at all. only leveled node |
19:19 |
Krock |
node level is param2, liquid state is param2 |
19:19 |
sfan5 |
you can also fix both leveled falling nodes and rework leveled nodes |
19:19 |
Krock |
hence add_node_level could be abused for non-levelled nodes (not checked9 |
19:20 |
Krock |
*) |
19:20 |
sfan5 |
Wuzzy: setLevel supports it https://github.com/minetest/minetest/blob/master/src/mapnode.cpp#L615 |
19:20 |
sfan5 |
I don't expect anyone to use that, but it's possible |
19:20 |
Wuzzy |
so what? its the modder's fault then. docs clearly say its for leveled nodes only. |
19:21 |
sfan5 |
besides setLevel already handles LEVELED_MAX, it just doesn't work since s8 is limited to [-128, 127] |
19:21 |
Wuzzy |
yeah ? |
19:21 |
Wuzzy |
noticed that too |
19:21 |
Wuzzy |
i dont understand why it even returns rest |
19:21 |
Wuzzy |
its a setter function, and hte caller already knows the value in advance, so... |
19:22 |
sfan5 |
it does so that addLevel can work |
19:22 |
Wuzzy |
i consider adding liquid level support to add_node_level to be a feature request then |
19:22 |
Krock |
sfan5: https://github.com/minetest/minetest/issues/9497#issuecomment-626894287 isn'tr this a hex value? |
19:23 |
sfan5 |
in gdb? I don't think so |
19:24 |
Wuzzy |
actually, i don'T feel comfortable with adding liquid support for add_node_level... liquid level and leveled level are two entirely different things. might be not the smartest to conflate them both... not sure |
19:24 |
Krock |
for exceptions I've seen hex values, thus I wasn't sure about this case |
19:24 |
sfan5 |
then don't, I would prefer that it still works though even if it's not documented :) |
19:25 |
sfan5 |
Krock: IIRC the hex values generally appear with __func__ or __FUNCTION__ which are used in exceptions here and there |
19:25 |
Wuzzy |
the alternative would be adding a new function with different name. not sure if its any better... |
19:25 |
Wuzzy |
I am generally against adding undocumented behavior just for teh lulz |
19:25 |
Wuzzy |
this is even worse |
19:26 |
sfan5 |
adding? you mean preserving |
19:26 |
Wuzzy |
preserving? |
19:26 |
Krock |
sfan5: okay thanks. Will keep an eye on that for the next cases |
19:26 |
Wuzzy |
this feature never existed to begin with |
19:26 |
sfan5 |
yes it did? you can start MT right now and call add_node_level on a liquid node |
19:26 |
Wuzzy |
... |
19:27 |
Wuzzy |
except that it was never officially supported for that |
19:27 |
Wuzzy |
lol you can call add_node_level on all nodes, really. but currently it only makes sense for leveled |
19:27 |
sfan5 |
yes but you're not "adding" undocumented behaviour if that behaviour already exists |
19:28 |
Wuzzy |
the problem is that current behavior with liquids is broken because there are no bound checks |
19:29 |
Wuzzy |
ok its not really a problem since its unofficial anyway ? |
19:29 |
sfan5 |
it works just fine if you stay in the range of liquid levels [0, 8] |
19:30 |
Wuzzy |
except if you do add_node_level(pos, 4) on a node that already has level 7 |
19:30 |
Krock |
will merge #9462 and #9833 in 10 minutes |
19:30 |
sfan5 |
no, that works correctly, setLevel() has bounds checks |
19:30 |
ShadowBot |
https://github.com/minetest/minetest/issues/9462 -- Add support for statbar “off state” icons by Wuzzy2 |
19:30 |
ShadowBot |
https://github.com/minetest/minetest/issues/9833 -- Damage texture modifier by appgurueu |
19:30 |
sfan5 |
anyway back to the topic: leveled nodes should have a max level specified in their nodedef so that they are actually useful |
19:30 |
Wuzzy |
Yes. |
19:31 |
Wuzzy |
I propose for it to default to 64 because double-high collisionboxes need extra-careful work to deal with |
19:31 |
Wuzzy |
oh wait |
19:31 |
Wuzzy |
crap we cant do that ? |
19:32 |
Wuzzy |
we already have a "default" ... implicitly. and its 127 |
19:32 |
Wuzzy |
Unless we go for 6.0.0 ? |
19:32 |
Wuzzy |
Let's release 6.0.0 only to change the default of max. leveled nodes. That's the best idea ever!!!1!1 |
19:32 |
sfan5 |
sounds good |
19:33 |
Wuzzy |
also lets throw in boost while we're at it |
19:33 |
Wuzzy |
anyway. leveled max value is good idea |
19:34 |
Wuzzy |
how would you actually enforce the max tho? |
19:34 |
Krock |
in setLevel |
19:34 |
Wuzzy |
re-interpret the param2 value? Or just prevent the param2 value from growing to large with the helper functions? |
19:34 |
sfan5 |
change the setLevel and addLevel arguments to s16 |
19:34 |
sfan5 |
and of course work with s16 internally where necessary |
19:35 |
sfan5 |
then you don't need any additional code in addLevel |
19:35 |
sfan5 |
and if you want you can clamp the range of l_add_level arguments to [0, 127] (so nobody tried to add 65530 levels) |
19:35 |
sfan5 |
tries* |
19:36 |
Wuzzy |
so in theory, a node with a max leveled can still exceed the maximum, but only with e.g. set_node |
19:37 |
Wuzzy |
But I think this is OK |
19:37 |
Wuzzy |
Let's just say if this hapens, we blame it on the modder ? |
19:38 |
sfan5 |
Wuzzy: can you reduce the empty space in the right side of the world creation dialog? (context: #9254) |
19:38 |
ShadowBot |
https://github.com/minetest/minetest/issues/9254 -- Add mapgen settings in world creation dialog by Wuzzy2 |
19:38 |
sfan5 |
it's a bit much imo |
19:38 |
Wuzzy |
sfan5: sure that its enough to only add the max_leveld check in setLevel? not that we're overlookng something... |
19:39 |
sfan5 |
no that's fine |
19:39 |
Wuzzy |
??????? |
19:39 |
Wuzzy |
why do you believe there is empty space O_? |
19:39 |
Wuzzy |
oh |
19:39 |
Krock |
git am shows them as warning, usually. |
19:39 |
Wuzzy |
because there is lol |
19:39 |
Krock |
merging..(2) |
19:40 |
sfan5 |
you can grep the code for CPT2_LEVELED in the engine or "leveled" in builtin to ensure you don't miss anything |
19:40 |
Wuzzy |
good point |
19:40 |
Wuzzy |
actually, i like to keep the spacing. reason: translations need space to breathe |
19:41 |
Wuzzy |
i think the biggest problem with many of the menus in the main menu is that everything is crammed in the smallest possible space. i think this is misguided design |
19:41 |
sfan5 |
I have marked the empty space for your convenience ;) https://a.uguu.se/pgL64bceqLrp_.png |
19:41 |
sfan5 |
well you don't have to remove it entirely, but it's a bit much for me |
19:41 |
sfan5 |
I might have a lower-than-default font size configured though... |
19:41 |
Wuzzy |
yeah, no. i gonna keep it. never underestimate the verbosity of other languages |
19:42 |
Wuzzy |
maybe, just maybe i change my mind if more people argue in favor it. |
19:43 |
Wuzzy |
I think this is ultimately just nitpicking and a matter of taste |
19:43 |
sfan5 |
there is really no reason text there should be long in any language... |
19:43 |
sfan5 |
maybe halve the space? |
19:43 |
Wuzzy |
then English would just barely fit, and then translators will DEFINITELY struggle |
19:44 |
Wuzzy |
wait. u mean cut 50% of the empty space? hmmmmm |
19:44 |
sfan5 |
yes |
19:44 |
Wuzzy |
hmmmmmmmmmmmmmmmmmmmmmmm |
19:46 |
Wuzzy |
actually i just realized that formspecs suck. our menus should grow dynamically, actually. the fact we even have to argue about size speaks volumes. |
19:46 |
sfan5 |
rough visual demonstration in gimp https://a.uguu.se/ocFT7snopVyo_.png |
19:46 |
Wuzzy |
yes yes i get it |
19:46 |
sfan5 |
yes formspecs suck but I don't think you want to wait until they stop sucking for this feature |
19:46 |
Wuzzy |
? |
19:47 |
Wuzzy |
I'd say better safe than sorry. i dont want to piss off translators ? |
19:48 |
|
Jordach__ joined #minetest-dev |
19:48 |
Krock |
"Frei schwebende Welten (experimentell)" even this level of verbosity would still fit so it's gonna be fine |
19:48 |
Wuzzy |
german's not the most verbose language tho. far from it |
19:49 |
Wuzzy |
So what cut do you suggest? 10% 25% 50%? |
19:52 |
Krock |
tend towards the 50% above |
19:53 |
|
Jordach joined #minetest-dev |
19:55 |
Wuzzy |
alrigh, fine... |
19:56 |
Wuzzy |
sfan5: Krock: Anything else I need to do with this PR? (#9254) or is this the last thing? |
19:56 |
ShadowBot |
https://github.com/minetest/minetest/issues/9254 -- Add mapgen settings in world creation dialog by Wuzzy2 |
19:56 |
Krock |
you have my approval, what more do you need? |
19:56 |
Wuzzy |
lol oops |
19:57 |
Wuzzy |
sorry. shoudl only have gone to sfan then :% |
20:09 |
|
proller joined #minetest-dev |
20:10 |
sfan5 |
Wuzzy: no thats the last |
20:10 |
sfan5 |
assuming I don't find a bug while testing it |
20:13 |
sfan5 |
hm I don't really like how settings are persisted in minetest.conf and there is no easy way to reset them to default |
20:13 |
sfan5 |
but that's not new so that doesn't matter |
20:14 |
Wuzzy |
sfan5: ok 50% reduction doesnt work ? |
20:14 |
sfan5 |
is there any reason to have the "mapgen-specific flags" label? the difference doesn't really matter to the user |
20:15 |
Wuzzy |
the reason is that paramat wanted me to do it ... |
20:15 |
Wuzzy |
sfan5: try selecting v6. the checkbox labels become longer |
20:16 |
Wuzzy |
Biomes drop-down also very _barely_ will fit in |
20:16 |
Wuzzy |
"Temperate, Desert, Jungle, Tundra, Taiga" |
20:16 |
sfan5 |
ugh |
20:16 |
sfan5 |
the (no effect on trees/junglegrass) seems superfluous when there clearly is a separate checkbox |
20:17 |
Wuzzy |
indeed |
20:17 |
Wuzzy |
this can be dropped safely |
20:17 |
Wuzzy |
but the dropdown is a different beast |
20:18 |
sfan5 |
is the dialog size at least reduced when game settings cause the entire right side to be empty? |
20:19 |
Wuzzy |
no |
20:19 |
Wuzzy |
this is only the case with singlenodes or a special game setting anyway, that the right size is completely empty |
20:20 |
Wuzzy |
also, resizing the formspec on the fly would lead to annoying "jumping" of widgets, which is not userfriendly |
20:20 |
sfan5 |
I agree but that makes dynamic sizing according to text length complicated |
20:21 |
Wuzzy |
or do you mean dynamic resizing of the whole thing when the Minetest window is resized? |
20:21 |
sfan5 |
no |
20:21 |
Wuzzy |
because this is certainly possible |
20:21 |
sfan5 |
for fractal the "Terrain" flag is badly named |
20:21 |
sfan5 |
the tooltip explains it but it's very nonobvious |
20:22 |
Wuzzy |
i agree |
20:22 |
Wuzzy |
i just copied the flag name, lazy lazy me |
20:22 |
Wuzzy |
hmmm let us think of a better name... |
20:22 |
sfan5 |
and maybe replace "Biome blend" with either "Biome blending" or "Blend biomes" |
20:23 |
Wuzzy |
"Non-fractal terrain"? hmm |
20:23 |
Wuzzy |
"Oceans and underground" idk |
20:23 |
Wuzzy |
yes the name "Terrain" is really misleading. as if the whole terrain disappears if you tick off this checkbox. ? |
20:24 |
Wuzzy |
"Additional terrain"? |
20:24 |
sfan5 |
sounds fine |
20:24 |
Wuzzy |
"Terrain that is not part of the fractal"? |
20:24 |
sfan5 |
flat mgv6 terrain sure looks interesting |
20:24 |
Wuzzy |
heh ? |
20:24 |
Wuzzy |
yeah its strange we had this v6 flag all these years but almost nobody actually used it ? |
20:25 |
sfan5 |
sloppy coding smh https://0x0.st/i_l8.png |
20:25 |
Wuzzy |
did you expect desert sand in the hole? ? |
20:25 |
sfan5 |
yes |
20:25 |
Wuzzy |
hmm its probably because the sand dropped into beach height |
20:26 |
Wuzzy |
but nobody wants to touch v6 anymore cuz compability. i can almost understand it |
20:27 |
Wuzzy |
sfan5: try playing around with mapgen aliases, by e.g. using the same node for differnt aliases. hilaritiy ensues |
20:27 |
Wuzzy |
turn tree and leaves into stone for example. (not sure if this triggers the weirdness) |
20:27 |
sfan5 |
the mapgen relies on placed nodes to indicate certain things and gets confused, right? |
20:28 |
Wuzzy |
idk, but yes this is also what i guess |
20:28 |
Wuzzy |
apparently v6 just assumes that all aliases are unique |
20:28 |
Wuzzy |
similar to how valleys assumes that river water and water are 2 different nodes |
20:29 |
Wuzzy |
if you try to use the same node for both, biomegen gets confused and spawns the top node in rivers (instead of the riverbed node) |
20:29 |
sfan5 |
yeah this seems like bad design |
20:30 |
Wuzzy |
i reported this years ago but it was shut down iirc. apparently this is one of the "acceptable bugs". idk. lol |
20:31 |
sfan5 |
I think it should be fixed some day if possible |
20:32 |
Wuzzy |
"additional terrain" ... still not too happy with that. still kinda vague. but eh. we have tooltip |
20:32 |
Wuzzy |
Bonus terriain... nah |
20:32 |
Wuzzy |
Extra terrain... |
20:32 |
Wuzzy |
hmmm i guess i just pick additional terrain. its good enough |
20:36 |
Wuzzy |
> is there any reason to have the "mapgen-specific flags" label? the difference doesn't really matter to the user |
20:36 |
Wuzzy |
I agree this is dumb, but paramat thinks differently ? |
20:36 |
Wuzzy |
I also dont like it as its too technical |
20:39 |
sfan5 |
> However, i can see the advantage of setting mapgen flags in world creation, as long as these only apply to the world then created and are not written to .conf (is this the case?) |
20:39 |
sfan5 |
(from paramat's first post) |
20:39 |
sfan5 |
was this ever addressed? |
20:40 |
sfan5 |
"Ok, this may be unavoidable, so is probably fine." ah |
20:40 |
Wuzzy |
it is written in .conf |
20:41 |
Wuzzy |
but the way i understand it, .conf is only read for new worlds |
20:41 |
sfan5 |
sure but |
20:41 |
Wuzzy |
it is ignored for existing worlds. so dont worry, your existing worlds wont get destroyed by this |
20:41 |
sfan5 |
<sfan5> hm I don't really like how settings are persisted in minetest.conf and there is no easy way to reset them to default |
20:42 |
Wuzzy |
there is a way to reset them to default. All Settings, click "reset" |
20:42 |
Wuzzy |
although this button is almost a lie |
20:42 |
sfan5 |
I can hardly take that suggestion seriously |
20:42 |
Wuzzy |
it doesn't delte the setting from .conf, it instead writes the default value explicitly into .conf |
20:43 |
Wuzzy |
i agree that All Settings hardly counts as "easy way to reset" |
20:44 |
sfan5 |
suggestion "(no effect on trees and jungle grass created by v6)" -> "(does not affect trees and jungle grass)" |
20:44 |
Wuzzy |
problem: game might spawn its own trees |
20:44 |
Wuzzy |
so saying "no affect on trees" is a lie |
20:45 |
Wuzzy |
or is this only about length? |
20:46 |
sfan5 |
the "by v6" felt unnecessary |
20:46 |
sfan5 |
maybe keep it idk |
20:46 |
Wuzzy |
sadly neccessary or otherwise i am a liar ? |
20:47 |
Wuzzy |
so the longest string in the right side is the biome dropdown in v6. i'm afraid it cant be shortened reasonably ? |
20:48 |
Wuzzy |
unless we go for "All Biomes" but i don't like that. I want the player to know what biomes exist |
20:50 |
Wuzzy |
so i guess we have to keep the current width... unless you have a better idea how to shrink the biome dropdown... or you no longer want me to shrink it anymore |
20:52 |
sfan5 |
the menu can't be shrunk so I am obviously not requesting it to be shrunk anymore |
20:54 |
Wuzzy |
about leveled: "the function can only return unsigned numbers so this doesn't make sense" |
20:54 |
Wuzzy |
that's why i negated the number |
20:55 |
Wuzzy |
so if you do -50 to level 10, you get returned 40 as rest. ok its a bit weird that the number is positive tho... |
20:55 |
Wuzzy |
should it just return 0? or change datatype to make it work? |
20:56 |
Wuzzy |
unsigned → signed |
20:56 |
sfan5 |
don't mind that comment and do all other changes first |
20:57 |
Wuzzy |
id rather fix everything at once instead of requesting a re-review for every single change ... |
20:57 |
sfan5 |
just pretend that review comment doesn't exist |
20:58 |
Wuzzy |
uh ... lol? |
20:58 |
Wuzzy |
o_O |
20:58 |
sfan5 |
http://irc.minetest.net/minetest-dev/2020-05-11#i_5684636 |
20:59 |
sfan5 |
do these first and if I still something wrong after that we can discuss the review comment |
20:59 |
sfan5 |
+see |
21:07 |
sfan5 |
merging game#2670 in 5 minutes |
21:07 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/2670 -- Update doors.it.tr by h4ml3t |
21:10 |
|
Miner_48er joined #minetest-dev |
22:28 |
|
GreenXenith joined #minetest-dev |
23:53 |
|
thePalindrome joined #minetest-dev |