Time Nick Message 00:04 paramat what i mean is that he can't maintain it yet (although i hope he learns more about core mapgen) if i partly assumed that it was my mistake 14:31 JDCodeIt Working on #7235 some more today - any ideas which areas to look into to see what may be holding up the memory between menu-game cycles? Some are probably intended (allocating a Driver object, etc). Also, there is clearly something continuing to add to the memory allocation each cycle. Not quite sure how t find this one. 14:31 ShadowBot https://github.com/minetest/minetest/issues/7235 -- Memory is not freed when exiting a world back to the main menu 16:24 JDCodeIt Yow, the hard part about tracking the allocation made in Irrlicht is that almost every ::get*() function in the library has an implicit grab() which increases the reference count. So matching get() and grab()'s with drop()'s is difficult at best. 16:27 JDCodeIt This line of code increments the ref counts on two objects (scene manager and IAttribute) - but if we don't save the pointer, we have no chance to drop() later. RenderingEngine::get_scene_manager()->getParameters()->setAttribute(...) 16:34 rubenwardy not very RAII 16:44 JDCodeIt True - I guess was some sort of efficiency thing to only try to allocate it once in Irrlicht, the dispose it only when everybody was done with it. 16:50 JDCodeIt In principle would you say that if a class takes a copy of a pointer from Irrlicht (reference counted), it should be responsible to drop() it on destruction? There are some caching objects outside the class, which could be exempt. 16:50 JDCodeIt or let's say on or before destruction 17:12 JDCodeIt There are instances where member variables don't start with m_ - should we aim to clean these up, or is there some other naming convention in play? 17:13 rubenwardy opposite 17:13 rubenwardy http://dev.minetest.net/Code_style_guidelines#Miscellaneous 17:14 rubenwardy huh, misread 17:14 rubenwardy " Use of Hungarian notation is very limited. Scope specifiers such as g_ for globals, s_ for statics, or m_ for members are allowed. The prefix m_ is discouraged for public members in newer code as it is a part of the class' interface, but sometimes needed for consistency when adding a member to older code. " 17:58 JDCodeIt OMG... Irrlicht increases the ref count on only _some_ get*() functions. This is going to be fun to figure out. 19:52 paramat rubenwardy sorry i misread your comment, i agree with you (and Shara) we need mapgen warnings in menu :] 19:55 Shara paramat: yes. Just some indicator of which are stable and which are not. 19:57 Shara You just cannot expect most users to have read things on the forum 19:57 paramat i'm not volunteering though, busy enough and not my speciality (no idea how to do it) 19:57 rubenwardy it'll be a one line change - if the selector already supports titles 19:58 paramat yeah it's another example of poor docs 19:58 rubenwardy overwise you'll have to add titles 19:58 rubenwardy well no, it's not documentation 19:58 Shara I'd even offer to look but free time is feeling like a myth right now 19:58 rubenwardy Valleys [Unstable] 19:58 Shara Yes 19:58 paramat i mean, poor informing 19:59 paramat ok i'll do it i can, will look 19:59 Shara I'd have had serious thoughts about using the mapgen if I had know the main feature of it (for me anyway) would get changed 19:59 rubenwardy how bad will the breakeage be? 19:59 Shara But only realised a couple months after starting the server 20:00 paramat yeah =/ 20:00 paramat a few flat walls in giant caverns, no more (rare) liquid sources in tunnels 20:01 rubenwardy that's less bad than surface breakage 20:01 rubenwardy like what has happened in v6 a few times 20:01 Shara paramat: are the valleys caves and the new giant caves in other mapgens really the same? 20:01 Shara (apart from location obviously) 20:01 rubenwardy also, don't caves over generate? So would it affect existing caves? 20:02 Shara I'm just asking because I've never yet seen v7 produce something that looks like the valleys caves 20:02 rubenwardy or is it just v5/6/7 that do that 20:02 paramat same generation, same squashed 3D noise 20:02 paramat the giant caverns don't overgen 20:03 paramat the valleys caverns were an exact copy of my lua mod, then the new caverns added to v5, v7 etc. were also a copy of that mod 20:04 Shara The valleys giant caves have a certain kind of structure that I can't seem to find in v7. Maybe it's just random luck though 20:05 paramat v7 has very similar caverns, but they're just smaller due to parameters. in that PR i have maintained the scale of previous mgvalleys caverns 20:08 paramat so, in PR 7244 the new caverns have the same xyz noise spreads as before, so size and shape character will be identical, just not location for half of the caverns (due to an added abs()). a half of caverns will be in identical locations an d almost seamless 20:10 paramat hehe 2013 #1033 20:10 ShadowBot https://github.com/minetest/minetest/issues/1033 -- Mapgen selection needs info text 20:11 paramat typical 22:03 ashtrayoz3 If people like documentation, then perhaps a review of #7231 is in order :-) 22:03 ShadowBot https://github.com/minetest/minetest/issues/7231 -- Add "not tool" function to extended drop tables, document how extended drop tables work. by ashtrayoz 22:03 sfan5 people like documentation, but nobody likes reviewing PRs 22:05 ashtrayoz3 Well, less of a review actually, more of a comment on a question. I have proposed syntax, if people agree on the syntax, then I will write the code. 22:13 JDCodeIt Renderer question - how can we call getMaterialRenderer (in clinet map) if we never called addMaterialRenderer? How do the renderers get created? 22:14 JDCodeIt Reason I ask, is that this renderer count is growing with each successive game-menu cycle. In my small test, it goes up by 20 each cycle. 22:15 Shara ashtrayoz3: I started looking at it this morning, but been a bit low on time and it's not something I'm familiar with 22:20 Shara From skimming, and keep in mind I might have missed something, it almost seems like you're making a case for being able to use a group for a tool type 22:20 ashtrayoz3 Groups would be a possible feature. The code already has string.find expressions, which in most cases is as good as groups. 22:21 ashtrayoz3 What the code currently lacks is negative expressions. Mainly because Lua does not have negative expressions. 22:21 Shara If I want any one of a certain thing to work in a recipe I'd put group:thing. So when I see you listing all those shovels I just want to see group:shovel 22:23 Shara if you start trying to match strings, well, what if someone named it a spade? 22:23 paramat testing maps posted here https://github.com/minetest/minetest/pull/7244#issuecomment-381768542 looks like i unknowingly fixed a bug too 22:24 ashtrayoz3 I think that horse has already bolted. The current code allows you to match lua patterns. 22:25 ashtrayoz3 There are good things and bad things about groups. For example, there is no shovel in default which is a member of group:shovel. 22:25 ashtrayoz3 But every shovel in default matches "default:shovel_". 22:26 ashtrayoz3 On the other hand, no shovel in amg_tools matches "default:shovel_", so then you get into pattern golf. 22:26 Shara ashtrayoz3: adding a group to MTG for it would be easy 22:27 ashtrayoz3 Perhaps you want ":shovel_", but perhaps that will match things that you do not want. So groups make sense. 22:27 Shara Food groups were recently added to MTG because a valid use was identified for it, for example 22:27 ashtrayoz3 Well, adding groups for lots of tools is a feature that I would support. 22:27 Shara If you go "shovel_", what if a mod named something "super_shovel"? 22:28 ashtrayoz3 But as I said, the pattern matching code is already master, and has been for years. The largest part of this pull request is actually documenting current behaviour (as SmallJoker had to point out to me :-) 22:28 Shara I doesn't seem like you really reached the point of this being just about documentation yet though :) 22:29 ashtrayoz3 This is currently a documentation only pull request, with a comment at the bottom about "but what should we do next?" 22:29 ashtrayoz3 So, based on feedback, I can go one of two ways: 22:29 Shara Reading the actual doc PR this morning made my head hurt in all honesty 22:30 Shara But that's not your fault 22:31 ashtrayoz3 1: add changes to item.lua to implement the "negative match" items in my last comment 22:31 ashtrayoz3 2: leave the "negative match" stuff as a comment in the discussion and change the name of the pull request back to what it was 22:33 ashtrayoz3 If I do 2, then I would start another pull request with changes to item.lua to implement whatever syntax was decided upon (should it be "?" or "^" even "<" ?) 22:33 ashtrayoz3 Along with documentation of that, of course :-) 22:34 ashtrayoz3 Investigating this has led me to find another "missing feature", as well as a bug. 22:34 ashtrayoz3 The new feature is in #7242, and is just rearranging existing code. 22:34 ShadowBot https://github.com/minetest/minetest/issues/7242 -- Make extended drop table code available for mods to use for craftitems and tools. by ashtrayoz 22:35 ashtrayoz3 I have written a fix for the bug, but it modifies the code in 7242, so I am not starting a pull request for the bug fix until that is dealt with. 22:35 Shara Checking the PR 22:36 Shara Advance warning: I'm a fussy proof-reader when it comes to docs :P 22:38 ashtrayoz3 And I tend to be an optimist, so I hope your fussiness improves my documentation ;P 22:43 Shara Hmm, so inside drop = {} we have items ={}, inside which we again have items = {}? 22:44 ashtrayoz3 Yes. I find it confusing, but that is the way that the current code works. 22:44 Shara No wonder my head hurt this morning 22:53 Shara Made some comments for you anyway. I hope it helps a bit 22:54 ashtrayoz3 I will check them out. 22:55 ashtrayoz3 At first glance, the comments look good. I will update the pull request later today. 22:56 ashtrayoz3 Do you have any opinion on what the syntax for "not" should be? 22:58 Shara Unsure what's best there. 23:00 ashtrayoz3 Well, if nobody expresses an opinion, then I get to choose :-) 23:03 rubenwardy don't bundle unrelated changes into PRs 23:03 rubenwardy as much as you can avoid that 23:19 ashtrayoz3 rubenwardy: I am thinking of moving the "add a NOT operation" into a separate pull request. SmallJoker has suggested doing two different ways. If the issue of syntax can be settled, I will probably revert this pull request back to "document the current behaviour" 23:45 ashtrayoz3 Updated #7231. 23:45 ShadowBot https://github.com/minetest/minetest/issues/7231 -- Add "not tool" function to extended drop tables, document how extended drop tables work. by ashtrayoz