Time Nick Message 00:50 rubenwardy #5154 00:50 ShadowBot https://github.com/minetest/minetest/issues/5154 -- Add ItemStack key-value meta storage by rubenwardy 00:50 rubenwardy (wip) 00:58 sofar ahhh man that makes me feel all warm and fuzzy on the inside 01:00 sofar rubenwardy: wait, did that just make the lua api half as easy? 01:00 sofar sorry, double as easy? 01:01 rubenwardy well, hopefully now all meta things should look the same 01:01 rubenwardy +/- inventory 01:01 rubenwardy for example, player meta should be ported to this 01:01 rubenwardy and mod meta 01:01 rubenwardy :D 01:01 sofar the patch series is really quite nice 01:02 sofar you did exactly the right steps I think 01:02 sofar kept but deprecated the old interface 01:03 sofar should port the damn description-in-itemstack-meta patch over, to test :D 01:04 rubenwardy well, I'm still debugging currently 01:04 rubenwardy doesn't actually work yet :D 01:04 rubenwardy itemmetatest/init.lua:10: calling 'get_int' on bad self (NodeMetaRef expected, got userdata) 01:04 rubenwardy ah yeah 01:04 rubenwardy grr 01:05 rubenwardy basically, in eg: l_get_int() it calls checkobject to make the ref 01:05 rubenwardy the problem is that checkobject isn't polymorphic as it's a static member 01:05 rubenwardy and you don't actually have a ref yet 01:05 rubenwardy hmmm 01:06 rubenwardy this sucks 01:06 rubenwardy https://github.com/minetest/minetest/pull/5154/files#diff-0dca00357942e57530fc94c52b90f7f5R30 01:07 rubenwardy forgot about that 01:08 rubenwardy basically, I need to check the userdata type is a class that derives from MetaDataRef 01:08 rubenwardy before casting to MetaDataRef 01:09 rubenwardy anyone know lua? 01:09 rubenwardy *the Lua C API 01:19 rubenwardy will people shout at me if I just check for *MetaRef (ie: NodeMetaRef, ItemStackMetaRef) in MetaRef::checkobject 01:19 rubenwardy :D 01:19 sofar sorry, you're far exceeding my c++ affinity :) 01:20 sofar I understand the concepts, but I'm like an elephant in a porcelain store when it comes to implementation 01:36 rubenwardy that's basically me right now 02:00 rubenwardy ok, it now works 02:00 rubenwardy wait, it shouldn't work 02:04 rubenwardy well, I'll fixing this for tomorrow 02:04 rubenwardy gtg 06:12 nore I must say I like this pr :) 06:12 nore need to read it a bit more, but that's what I was expecting from it 07:58 nrzkt #5131 is now ready for merge to master, please review :) 07:58 ShadowBot https://github.com/minetest/minetest/issues/5131 -- Add ModStorage Lua API by nerzhul 08:11 kaeza +1 API wise. I don't like the use of JSON too much, but that's just me 08:12 nrzkt kaeza, you will not see it :) 08:13 kaeza ofc 08:13 kaeza ... 09:15 paramat nore sfan5 is game#1540 ok? any thoughts on game#1475 ? 09:15 ShadowBot https://github.com/minetest/minetest_game/issues/1540 -- Trees: Add 'snowy' group for pine sapling snow detection by paramat 09:15 ShadowBot https://github.com/minetest/minetest_game/issues/1475 -- Ores: Add silver sand blob ore, relocate other blob ores by paramat 09:15 sfan5 snow thing lgtm 09:16 paramat ok i'll merge that and game1534 later 09:21 paramat unless objections i'll merge #5148 in a few hours since it's simple, well tested and is mapgen stuff i fully understand 09:21 ShadowBot https://github.com/minetest/minetest/issues/5148 -- Mgvalleys: Fix missing decorations and incorrect function order by paramat 09:37 nrzkt sfan5, nore: can you review 5131 please ? 09:37 sfan5 later 09:38 nore nrzkt: looking but I don't have much time 09:38 nrzkt no problem :) 09:38 nore can we merge #5143 btw? 09:38 ShadowBot https://github.com/minetest/minetest/issues/5143 -- Fix anticheat resetting client position after the client is teleported. by Ekdohibs 09:39 nrzkt nore: go 09:41 red-001 could someone review #5152? 09:41 ShadowBot https://github.com/minetest/minetest/issues/5152 -- [CSM] Add `get_wielded_item` by red-001 09:41 nore nrzkt: I don't have the time to test it, but 5131 looks good 09:43 nrzkt great i hope sfan5 and sofar could finish the review and approve permitting to merge and add client side part to CSM :) 09:46 paramat i can merge 5143 when i merge my mapgen PR 09:48 red-001 also 5149 and 5142 have all the problems mentioned in comments fixed 12:24 red-001 are there any issue with #4962 apart for the hacked together way that it uses to stop minetest from being crashed by unicode in the config files? 12:24 ShadowBot https://github.com/minetest/minetest/issues/4962 -- Save the name of the world in world.mt and remove invalid characters from the directory name by red-001 12:26 Zeno` why is '_' a bad character? 12:26 Zeno` also... 12:26 Zeno` c == ' ' is that necessary? 12:28 Zeno` isalnum is already checked. ' ' is almost never correct even when it's required (isspace() is better) 12:28 VanessaE don't take - and _ out of worldnames, that will break my servers! 12:28 Zeno` yeah 12:28 Zeno` I can't see why they should be illegal 12:28 paramat underscore seems acceptable to me 12:28 red-001 bad_char is a bit of a misleading name 12:28 VanessaE - works fine in filenames too 12:29 paramat better than a space :] 12:29 paramat yes 12:29 VanessaE (as long as it's not the first char, then you need an escape) 12:29 red-001 - , _ and space are allowed 12:30 red-001 unless I messed up and somehow it excludes them 12:30 Zeno` well fix the function name in that case :P 12:30 paramat but slashes should be disallowed? 12:31 VanessaE only in unix environments 12:31 VanessaE I think they're valid in windows 12:31 Zeno` it doesn't make sense to me atm. And doesn't even really match the conditions checked for if it (logic) was reversed 12:31 red-001 if bad_char returns true then that charcter is removed 12:32 Zeno` so '-' and '_' are removed? 12:32 red-001 no as the condition is reversed 12:32 paramat oh, 'bad_char' returns NOT - _ or space 12:33 red-001 !(isalnum(c) || c == '-' || c == '_' || c == ' '); 12:33 Zeno` it seems wrong then 12:33 Zeno` apply DeMorgan's laws 12:33 paramat should spaces be allowed? they seem problematic 12:33 Zeno` don't use c == ' ' anyway. Use isspace 12:33 VanessaE spaces should be allowed, but let the user/admin/whoever decide how to deal with them 12:34 Zeno` but this bad_char function seems wrong 12:34 red-001 why so? 12:34 Zeno` well, not wrong per se 12:34 Zeno` just... hmm... how to explain 12:35 red-001 I'm pretty sure I tested the code to see if it worked 12:35 Zeno` well it does 12:35 Zeno` ok let it stay 12:36 Zeno` but why are you using c == ' '? 12:36 Zeno` because spaces are allowed but now newlines and tabs etc? 12:36 Zeno` s/now/not 12:37 * red-001 checks if tabs/newlins break filenames on windows 12:37 Zeno` they should not be allowed 12:37 Zeno` I'm not explaining myself well :) 12:37 Zeno` apply DeMorgan's laws 12:38 Zeno` and use what you end up with. It's possibly clearer 12:38 Zeno` if (!blah && !blah ...) 12:38 Zeno` err != 12:39 Zeno` this is why I don't like this simple function :( 12:39 * red-001 is now even more confused 12:39 Zeno` that's because the function is already confusing? 12:40 Zeno` even though it's simple? 12:40 paramat i'm ok with it now 12:40 red-001 is you would prefer if I got rid of the !(conditions) and replaced it with !condition || !condition ||...? 12:41 paramat 'good char' is 'alphanumeric or - or _ or ' ''. then ! it for 'bad char' 12:41 paramat seems simplest how it is 12:41 Zeno` the expression return !(isalnum(c) || c == '-' || c == '_' || c == ' '); is the same as return !isalnum(c) && c != '-' && c != '_' && c != ' '; 12:41 Zeno` I'm not sure what is clearest 12:42 red-001 well I can change it to the second one if you prefer it, I just found the other one less confusing 12:42 paramat how it is seems clearer and simpler to me 12:42 Zeno` I don't mind because I understand DeMorgan's laws, but when you look at the function name it's... I dunno 12:42 Zeno` I dunno... leave it 12:43 Zeno` personally I'd prefer the function to be called is_good_char() 12:43 red-001 not sure thats possible 12:43 Zeno` and then the logic used wherever would be if (!is_good_char()) { } 12:44 Zeno` because positive makes more sense to me 12:44 Zeno` if (!good_char(c)) { // reject } 12:45 Zeno` There is a kind of style guideline that suggests to use that approach as well (somewhere) 12:46 paramat the using bools in the positive sense thing 12:47 paramat 'good char' and using '!good_char' in input.erase() is ok, if it works 12:47 Zeno` correct 12:47 Zeno` most code uses the "positive" stance 12:47 * red-001 checks if that will work 12:47 paramat that's probably a little clearer 12:48 Zeno` not the isbad() kinda thing 12:48 Zeno` I dunno how to explain it. 12:49 paramat yes if it works i slightly prefer 'good_char' 12:50 red-001 ok testing if it will work 12:50 paramat so just move the '!' into input.erase() 12:51 red-001 I think windows doesn't allow spaces at the beginning of filenames 12:51 paramat that's a bad idea anyway 12:51 red-001 so should I filter spaces as well? 12:52 paramat personally i would disallow spaces anywhere 12:52 red-001 or just trim trailing and leading? 12:52 Zeno` it'll work 12:52 red-001 ohh I think I alrady trim trailing 12:52 red-001 just need to trim leading 12:52 paramat trim leading too at least 12:53 Zeno` I guess the reasoning is that is_good() is much easier to understand than !isbad() (eliminates the double negative) 12:54 red-001 yeah I prefer that too, I just though I couldn't pass !good_char to erase 12:54 Zeno` even though one of the "negatives" is english and not logic heh 12:54 Zeno` yep 12:54 Zeno` hmm 12:54 Zeno` well I leave it with you. If it's not possible it's not possible 12:56 red-001 well it seems to be building without errors 12:59 sfan5 red-001: added a review to your pr 13:01 sfan5 maybe it's just me but i often find the lack of attention to detail in PRs disturbing 13:01 sfan5 it seems like many PRs are made to "just work" and that's it 13:06 red-001 for the braces thing I usually do that on propose as it's less confusing 13:10 sfan5 it's not confusing at all imo 13:10 sfan5 but either way is fine according to the style guide 13:15 red-001 I suppose it's personal preference 13:20 nrzkt sfan5, Zeno`: #5153 needs review :D 13:20 ShadowBot https://github.com/minetest/minetest/issues/5153 -- Fix facedir_to_dir and wallmounted_to_dir for coloured nodes. by Ekdohibs 13:20 nrzkt oops 13:20 nrzkt #5131 13:20 ShadowBot https://github.com/minetest/minetest/issues/5131 -- Add ModStorage Lua API by nerzhul 13:20 nrzkt is seems travis is buggy 13:20 nrzkt builds doesn't start 13:27 red-001 sfan5, I can't use the ARRAYSIZE marco as its in keycode.cpp and not a some sort of utility file 13:27 sfan5 well then just copy it form there 13:27 sfan5 from* 13:29 Zeno` wait 13:29 Zeno` we have the macro in the .cpp file? 13:29 red-001 that seems counter-productive, code duplication and all 13:29 red-001 yes Zeno` 13:29 Zeno` ouch 13:29 Zeno` but... ARRSIZE is elsewhere? 13:29 Zeno` I thought we fixed this :( 13:30 nrzkt ARRLEN no ? 13:30 Zeno` maybe that's it nrzkt 13:30 red-001 so there is ARRSIZE and ARRAYSIZE? 13:30 nrzkt ARRLEN is an header 13:30 Zeno` we should do a grep of the src code and get rid of all this trash 13:31 sfan5 do this https://github.com/minetest/minetest/commit/4d4b8bb8a46b6472d86fa848954dbc26b4fadb50 13:31 sfan5 but with ARRAYSIZE 13:31 Zeno` yes 13:31 Zeno` ARRLEN is not a great name (it's not quasi standard) 13:32 Zeno` hmmm 13:32 Zeno` dunno what is though 13:32 Zeno` there is NELEMS 13:32 Zeno` it's all crazy 13:32 Zeno` but we should not have code all over the place doing the same thing 13:37 juhdanad May I try to implement #1367 or is someone already working on it? 13:37 ShadowBot https://github.com/minetest/minetest/issues/1367 -- Proper display of text on the surface of a node(box) 13:39 paramat please do 13:39 paramat i don't know of anyone working on it 13:44 paramat perhaps discuss your implementation in the thread, as there seems to be some controversy over how to do it 13:50 red-001 #5152 need another review 13:50 ShadowBot https://github.com/minetest/minetest/issues/5152 -- [CSM] Add `get_wielded_item` by red-001 13:55 juhdanad paramat: done! 14:00 paramat #5157 14:00 ShadowBot https://github.com/minetest/minetest/issues/5157 -- Objectpos over limit: Avoid error caused by sector over limit by paramat 14:57 paramat will merge #5143 #5148 in a moment 14:57 ShadowBot https://github.com/minetest/minetest/issues/5143 -- Fix anticheat resetting client position after the client is teleported. by Ekdohibs 14:57 ShadowBot https://github.com/minetest/minetest/issues/5148 -- Mgvalleys: Fix missing decorations and incorrect function order by paramat 15:11 paramat merging 15:15 paramat merged 15:19 juhdanad paramat, do you have further questions to my light spreading PR? Or do you think it is complete? 15:26 paramat erm, i still need to look through it (the first one), i doubt i will understand everything though 15:26 paramat i'd like to get those moving so i should review it 15:28 juhdanad paramat: I think this will help you understand: https://www.seedofandromeda.com/blogs/29-fast-flood-fill-lighting-in-a-blocky-voxel-game-pt-1 15:28 paramat is this more relevant for your 1st or 2nd PR? 15:29 paramat everyone, #4682 and #4967 are top priority for review 15:29 ShadowBot https://github.com/minetest/minetest/issues/4682 -- Fix water flooding onto lava by juhdanad 15:29 ShadowBot https://github.com/minetest/minetest/issues/4967 -- New bulk node light update by juhdanad 15:31 juhdanad It is relevant if you want to understand the basics of light spreading, which are used by both PRs. 15:31 paramat ah ok i have some understanding already 15:32 paramat will read the link thanks 16:47 red-001 fixed the issues with #4962 16:47 ShadowBot https://github.com/minetest/minetest/issues/4962 -- Save the name of the world in world.mt and remove invalid characters from the directory name by red-001 17:43 paramat will merge game#1475 game#1539 game#1540 in a moment 17:43 ShadowBot https://github.com/minetest/minetest_game/issues/1475 -- Ores: Add silver sand blob ore, relocate other blob ores by paramat 17:43 ShadowBot https://github.com/minetest/minetest_game/issues/1539 -- Creative: Cache creative mode setting by paramat 17:43 ShadowBot https://github.com/minetest/minetest_game/issues/1540 -- Trees: Add 'snowy' group for pine sapling snow detection by paramat 17:47 sofar paramat: 1539 == +1 from me 17:47 sofar paramat: 1540 == +1 from me as well 17:49 sofar paramat: 1475 == +1 from me as well 17:50 paramat thanks 17:54 paramat merging 18:01 paramat done 19:24 paramat will test and hopefully merge game#1542 tomorrow, too tired now 19:24 ShadowBot https://github.com/minetest/minetest_game/issues/1542 -- Mapgen: Dedicated registrations for mgv6 blob ores by paramat 21:04 sapier https://github.com/minetest/minetest/pull/5160 can someone plz approve that obvious bugfix? :-/ 21:08 nore sapier: what's the difference with #5151? 21:08 ShadowBot https://github.com/minetest/minetest/issues/5151 -- Tell on_punch to expect a return value by duane-r 21:08 sapier oh :-) none we can merhe 5151 too 21:11 sapier any objections to merging 5151? 21:12 nore sapier: none from me 21:12 nore merge it! 21:12 sapier ok on my way 21:40 red-001 does digging still generate cpu lag spikes? 21:41 kaeza sapier, adding a flag in `core.features` would have been nice... 21:42 kaeza re: on_punch thingy 21:42 sapier kaeza sorry didn't realize this as it was intended to not change behaviour for existing mods 21:43 sapier the whole (temporary) change in behaviour was a bug 21:43 kaeza sapier, no problems. it can still be added 21:43 kaeza but the problem is not the "temporary" bug 21:43 sapier true 21:43 sapier but not sure if it's worth it by now noone except of me did need it 21:43 kaeza the issue is: how do I know if returning true from on_punch will do anything useful? 21:44 sapier depends it's only usefull if you do something within the handler 21:44 kaeza I may want to handle damage myself if I can't guarantee the engine will handle it for me 21:44 sapier if you don't return anything or return false engin will do it 21:45 sapier so you can be sure engine does do anything but the other way round you can't 21:46 kaeza hm, true 21:46 sapier well of course we can add a feature flag for it but I'm not sure if it's enough difference to be worth it 21:47 kaeza it's fine either way I guess, in this case at least 21:48 kaeza sorry for the ping :) 21:48 sapier true, I'd be fine with both solutions too. especially as you already can detect the difference "damage" parameter is nil on old minetest versions 21:48 sapier of course that's not really a good way to detect 21:49 sapier no problem kaeza that's what irc is for discussing things to get best possible solution 21:49 kaeza ah the damage param. that could be useful I guess 21:49 kaeza and I disagree it's not the best solution 21:49 sapier and I didn't even thing about the feature flags so you're completely right to mention it 21:50 kaeza I think it's the perfect solution to detect new arguments in that case 21:51 sapier hmm not quite sure if I do understand correct. You want to say that detecting the change by checking the parameter is better then the feature flag? 21:51 kaeza see e.g. the new timed_out param to on_leaveplayer. you can detect older servers by doing if timed_out==nil 21:52 sapier true and additionaly it can't be inconsistent 21:53 kaeza hm, let's just wait until someone has a problem with the new API in practice 21:54 kaeza but it's nice to consider `features` to note changes in the API not otherwise detectable 21:54 kaeza (e.g. add_entity_with_staticdata) 21:54 sapier well it's an extension to be more precise :) 23:56 Fixer ~~~ fog in clouds in minetest please ~~~ 23:58 sofar maybe a color effect like underwater has? 23:59 Fixer yes, it is sufficient fog like effect 23:59 Fixer i don't even ask for fancy