Time Nick Message 01:13 Unarelith where is defined node destruction animation? is it usable for an entity too? 01:13 rubenwardy the particles? 01:14 rubenwardy or the cracks? 01:14 Unarelith both actually 01:14 rubenwardy I'm not sure, thought I'd ask that question though X 01:14 rubenwardy err, awks 01:14 rubenwardy *XD 01:14 Unarelith I'm trying to make an entity destruction look like a node destruction 01:15 Unarelith so if I can get the node destruction code to work with a wielditem entity, could be nice 01:28 Unarelith hmm, seems like it's in Game 01:28 Unarelith why :'( 01:34 Unarelith well, I don't think Game::handleDigging could work for entities unfortunately 01:36 Unarelith so the only way would be to have an invisible node around the entity to handle its destruction 01:37 Unarelith therefore, entities must be bound to their parent node 01:38 Unarelith and I don't see how to make this possible, that's why I tried to use entities as dynamic nodes 01:40 Unarelith if additional entity data has to be stored in nodes then I'm not familiar enough with the engine or the network protocol to add something like this yet 01:41 paramat the digging cracks are problematic anyway. they look wrong on some nodes, are stretched or distorted on others, and cause mesh updates. particles are much better 01:43 Unarelith the main issue here is that entities and nodes are so separated that a class which would be between them in term of features would mean copying code or doing a refactoring somewhere else 01:44 rubenwardy they are rendered completely differently, by necessessity 01:44 Unarelith I'm not really speaking about their rendering 01:46 Unarelith I'm speaking about an interaction between them, like a parent-child relationship 01:47 Unarelith but since entity attachment system is based on irrlicht scene graph, it can't be used here 01:51 T4im and what would that relationship stand for? 01:53 T4im i'm sure you don't want to create such a relationship just do add an animation to a model 01:54 Unarelith T4im, see #6963 01:54 ShadowBot https://github.com/minetest/minetest/issues/6963 -- Node bound entities 01:56 T4im so you want to serialize them per node instead of per mapblock? 01:59 Unarelith actually, I don't really know how to do that since I'm not familiar enough with the engine yet, I didn't even know that entities were stored in mapblock 02:02 T4im i thought they were serialized per mapblock; but don't take my word for it 02:08 Unarelith paramat, I didn't understand well the relation between #7908 and your last comment on this PR, could you explain please? 02:08 ShadowBot https://github.com/minetest/minetest/issues/7908 -- File 'client/game' splitted into new folder by Quent42340 02:08 T4im but it seems to me, that unless you plan to change node storage in a breaking way, you might be forced to store some reference into the node metadata, or create something similar to lookup entities for a node 02:13 paramat yeah my last coment there is not particularly relevant to that PR, more a general point about discussing how finely we split things 02:15 paramat but i somewhat like how easy it will make things to find. it's possible i'm over-estimating the number of small files that will be created 02:16 T4im the terasology thing seems hard to compare; java doesn't suffer from splitting files as much as c/c++ wrt to compiling/translating units 02:17 Unarelith paramat, I think you're over-estimating since some of them can definitely be packaged (for example ContentParamType, LiquidType, NodeDrawType, etc... belongs in the same file) 02:18 paramat yes i'm not certain those dev rules are actually relevant to what you are doing. i should perhaps delete that comment 02:18 Unarelith and there's a lot of files which won't move since they don't define any unit 02:19 paramat nice 02:20 Unarelith they're relevant to MT code though, the abstractions are globally bad in my opinion 02:21 Unarelith for a future PR, I prepared a splitting of ContentCAO + a refactoring of the visual system + clean separation of commands (all done and working already) 02:22 T4im i forgot, does minetest compile with lto? 02:26 T4im it appears not 02:26 T4im in that case that would make the better reason for not wanting too many files, paramat :D 02:30 Unarelith why? to reduce compilation time? 02:30 T4im to not waste optimization opportunities; the compilers can't really optimize across translation units 02:30 T4im split into too many files and things will slow down 02:32 Unarelith I don't see a practical case of optimization needed across translation units 02:33 T4im you don't see a practical case for optimizations? 02:34 * T4im is not sure what that means 02:35 Unarelith sorry, I meant that I don't see a case where this kind of optimization could be useful 02:35 Unarelith so please provide an example 02:35 T4im well, it's usually up to the compiler to find those cases; what functions to inline, what code to eliminate, when to unfold 02:39 T4im idk, lemme make something up then, you might have a loop somewhere that makes a function call into another translation unit; unless you optimize across that boundary, you'd be unable to inline that function, and if it ended up being unnecessary, you the compiler would not be able to remove it unless he'd be running with lto 02:39 T4im -you 02:40 Unarelith why would I be unable to inline that function? 02:41 T4im because you optimize before linking 02:45 Unarelith I don't really understand why exactly the compiler wouldn't choose to inline this function 02:46 T4im because it doesn't know its implementation at all 02:51 Unarelith ok, I misunderstood inline usage I guess 02:51 paramat as long as in-game performance isn't affected i don't mind slightly slower compiling 02:52 Unarelith actually, in-game performance COULD be affected, but not much really 02:52 T4im well the worst case would be running with an unoptimized/debug build 02:52 T4im :D 02:52 VanessaE someone merge my PR :) 02:52 paramat hopefully insignificant in-game performance change? 02:53 Unarelith yep, really insignificant 02:53 T4im idk, would have to be checked 02:54 Unarelith from what I saw in MT code, even with file splitting, units are still so big that it really won't be an issue 02:54 T4im for server processes that's usually unimportant anyway 04:08 paramat merging #7921 04:08 ShadowBot https://github.com/minetest/minetest/issues/7921 -- Slightly alter star appearence time and full brightness time by paramat 04:17 Unarelith ... really? https://github.com/minetest/minetest/blob/master/src/client/clientlauncher.cpp#L346 04:17 paramat merged 04:20 paramat since that code's beyond me, what's the issue? 04:21 Unarelith a dynamic allocation stored nowhere 04:22 Unarelith I guess there's a global somewhere to hold it with a "g_the_global = this" in the class and a "delete g_the_global" somewhere else 04:22 Unarelith but still, a line containing nothing but 'new MyObject' is a really bad practice 04:30 Unarelith ok seems like I was right. but ClientLauncher implementation doesn't make any change possible since I guess there's an ordering for the init calls 04:31 Unarelith so RenderingEngine has to be initialized at that specific moment, that's why it's not a member of ClientLauncher, hmm 04:31 Unarelith it's a bit dumb since ClientLauncher is definitely the owner of RenderingEngine 04:35 Unarelith core is a std::unique_ptr: https://github.com/minetest/minetest/blob/master/src/client/renderingengine.cpp#L119 04:35 Unarelith seems like someone didn't understood smart ptrs 04:41 Unarelith ok well I managed to fix this issue by adding a new member to "ClientLauncher" named "m_rendering_engine" and I swapped RenderingEngine constructor with a new function 'init' to keep init ordering 04:42 Unarelith could be in a future PR 04:47 Unarelith what do you think paramat? 04:53 paramat sorry, i have to go, but PRs are welcome 07:01 paramat merging trivial #7928 in 15 mins 07:01 ShadowBot https://github.com/minetest/minetest/issues/7928 -- Draw stars behind the moon by paramat 07:23 paramat merging 07:26 paramat done 11:03 ANAND Krock: How do you feel about renaming `DIG` and `PLACE` to `USE` and `SECONDARY_USE` respectively? 11:04 Krock no, that would only be confusing 11:04 ANAND But LMB and RMB do much more than just dig or place 11:05 ANAND "Click the place button to open this chest" 11:05 ANAND That sounds very confusing 11:05 ANAND AFK for a bit 11:16 Krock next question: which is the "use" key? I thought that would be "E" 11:33 ANAND Krock: It has been renamed `aux1` everywhere. 11:34 ANAND IIRC 12:42 p_gimeno wasn't DIG called PUNCH? 12:43 ANAND AFAIK, it was just called LMB 12:44 ANAND is* 12:47 nerzhul generic way can be nice 12:47 nerzhul :) 12:56 Unarelith guys seriously, review existing code you'll avoid a lot of issues and it'll be time saving later, there's a lot of unnecessary dynamic allocation, unused class members, bad access modifier usage (some protected members should be private) 12:58 Unarelith some parts in the code are really good, but other parts are just horrendous 13:12 nerzhul Unarelith, we already know that, you know, but we are in free software, not working everytime on the code, i have done many refactor since 3 years and i'm not the only guy 13:12 nerzhul the horrendous part is generally legacy don't touched part 13:35 Unarelith I understand that, though there is a lot of easy improvements, like GenericCAO visuals refactoring for example: https://github.com/Quent42340/minetest/commit/be967430a4824e6b0b88c8bf9c99385bf1c3085e 13:35 Unarelith or smart ptr usage/better variable lifetime handling: https://github.com/Quent42340/minetest/commit/68bb8b451f3fd88bcb6d52cfe6832e29fd13af6a 13:39 Unarelith I could make a PR for each of these fix but it would take so much time to get them merged 13:40 nerzhul yes then don't loose time 13:40 nerzhul we need function not refactor at this time, and bug fix 13:40 nerzhul the goal is to release ASAP 13:40 nerzhul code quality can wait after release 13:41 Unarelith then just merge existing PRs 13:42 Unarelith there's a lot of bugfixes/features there 13:43 Unarelith and my time is not wasted since my refactorings could be easily added as new PRs later, and I'm learning the codebase at the same time 13:43 Unarelith d'une pierre deux coups (j'ai pas la version anglaise sorry) 13:48 sfan5 merging existing PRs should be done far more often 13:49 sfan5 the problem here is the 2 approvals rule, which massively slows down the process when there's not enough devs with time 13:49 Unarelith this rule is good though 13:50 sfan5 it is yes, slowing down development is just an unfortunate side effect 13:51 nerzhul Unarelith: merge existing PR is not just merge, many are missing quality or are not complete 13:51 nerzhul also i'm french i understand what you tend to say but no :p 13:51 Unarelith but some looks like really small changes and are complete 13:52 Unarelith and btw I know you're french, that's why I said it like that 13:52 Unarelith though I didn't understood your "no" 13:58 Unarelith I think you should just find good C++/Lua devs (preferably contributors who know the code) and add them as coredevs since current coredevs don't have much time 13:58 Unarelith 5.0.0 could be out in less than a month that way 13:59 ANAND I agree. With the severe shortage of dev time, more coredevs would give MT a big push forward 14:01 Unarelith but the main issue is to find these good devs, and to ensure they've enough free time to work on MT 14:08 Krock 5.0.0 could be out in less than a month that way 14:08 Krock that's what we said in July 14:08 T4im :D 14:09 Unarelith Krock, and you have more coredevs than in July? 14:11 Krock not sure when Lohfhansl was added to the team.. recently. maybe it's one more now in that case 14:12 Krock rubenwardy: did numberZero decline or what's the status with him? 14:15 rubenwardy Waiting on celeron55 14:18 Krock ah, okay 14:19 Unarelith btw Krock, I don't think node-bound entities can be a thing right now 14:21 Krock do you think it needs too much refactoring? 14:21 Unarelith I think refactoring could be helpful to implement this feature but it's not the issue here 14:21 Unarelith entities and nodes are really separated 14:22 Unarelith storing entity data in node metadata is.. well, not really good 14:22 Unarelith and I don't know any other way to store it 14:23 Krock a new type :3 14:23 Unarelith definitely 14:23 Krock refactor the map format also, when you're already on the way :P 14:23 Unarelith you mean the map storage? 14:24 Krock yes 14:24 Krock merging trivial #7829 in ~10 minutes 14:24 ShadowBot https://github.com/minetest/minetest/issues/7829 -- fix spelling by kurzkopfgleitbeutler 14:25 Krock #7929 too 14:25 ShadowBot https://github.com/minetest/minetest/issues/7929 -- Remove unused main menu settings by pauloue 14:26 Unarelith that wouldn't really be refactoring, just format improvement 14:26 Unarelith and that kind of stuff breaks a lot of things 14:27 Unarelith so I assume this won't even be merged if I do a PR 14:28 Unarelith and btw, I can do 4 or 5 code improvement PRs per day (C++11 fix, unnecessary dynamic allocation fix, etc..) 14:29 Unarelith but giving the current PR acception rate, it's worthless 14:31 Unarelith anyway, I'm currently working all day on my fork and I plan to add all my code improvements to MT, but I'd really prefer working on MT directly 14:33 Krock but it's great to see such motivation to program :) 14:33 Krock or more important: time. 14:33 Krock merging.. 14:33 Unarelith I'm 100% free this month and MT is a really interesting project 14:33 Unarelith btw, I made that: https://github.com/Quent42340/KubKraft 14:34 Unarelith but MT is way more advanced so there's no point working on it now 14:35 Krock KRAFT 14:35 Unarelith don't laugh, I made up this name when I was 15 14:35 Unarelith so 7 years ago actually 14:38 Unarelith (yep I'm only 22 but I'm pretty sure everybody here already guessed that) 14:41 celeron55 code looks fairly similar to very early minetest 14:42 celeron55 it's refreshing to see a C++ one when everything today is made in java or some unnecessary bloatware 14:42 Krock but it's proprietary (no license included) 14:44 Unarelith yep, I don't really like license headers 14:45 T4im no LICENSE file either :p 14:45 Unarelith celeron55, really? that was what MT looked like in the beginning? 14:46 Unarelith T4im, this project is really small, I was working alone with no release planned, why would I care about a license? :') 14:47 Unarelith but the goal of this project was to make something really similar to MT 14:48 Unarelith so I decided to work on MT instead 14:48 celeron55 yes, really 14:48 Krock !tell paramat Would you please be so nice and have a quick look at #7847 ? It works, but are the defaults handled correctly there? Not sure how the flags are handled 14:48 ShadowBot Krock: O.K. 14:48 Krock thx ShadowBot 14:49 celeron55 it's a bit tidier than what i usually end up with 14:50 celeron55 and with some stylistic differences and back-end differences 14:50 rubenwardy in the words of somebody (probably hmmmm): celeron55 is very good at getting code working quickly 14:50 rubenwardy or something like that 14:50 rubenwardy it's been years 14:50 Krock will merge #7717 in ~10 minutes LGTM 14:50 ShadowBot https://github.com/minetest/minetest/issues/7717 -- document which formspec fields are sent by zeuner 14:52 Unarelith I'm not good at that at all, I usually spend hours to think about how to organize the code to work faster afterwards 14:52 Krock ^ if the wording should be improved, I can adjust that too in the next minutes 14:52 Unarelith I'm really lazy and KISS, DRY and SRP are my favorite programming principles 14:53 Krock Unarelith: organisation (including build system setup) are the most demanding because it's very hard to find the best solution 14:53 celeron55 Unarelith: that's like me 14:53 Unarelith btw, although SRP exists in MT, SRP is also a programming principle Krock 14:54 celeron55 (without the hours of thinking) 14:55 celeron55 so: KISS, DRY, SRP and avoid-premature-optimization 14:55 Unarelith Premature optimization is root of all evil. 14:55 Krock oh. not the auth mechanism then 14:55 Unarelith yep, Single Responsibility Principle 14:56 Unarelith but MT code goes against all these principles 14:57 Unarelith Unarelith: organisation (including build system setup) are the most demanding because it's very hard to find the best solution <= That's what I've worked on my whole life, so sorry if my current PRs are related to that topic but I think it's really important 14:59 celeron55 it's really hard to maintain any of those principles when multiple people are working on the same program 14:59 celeron55 i guess that's the main reason 14:59 * T4im has yet to see someone that intentionally likes unnecessarily complicated, repeating code split over several places that is fast in all the wrong places 14:59 rubenwardy that's supposedly what the review system is for 15:00 T4im it's probably just a matter of priorization sometimes 15:00 Unarelith celeron55, hmm I disagree if these principes are enforced in the reviews and the original code 15:00 rubenwardy the issue is defining the scope of things 15:00 rubenwardy for example, what is Game for? 15:00 Unarelith Game is a really bad example 15:00 Unarelith but take GenericCAO 15:01 Unarelith I splitted the animation part into GenericCAOAnimation and the visual part into IGenericCAOVisual and SpriteVisual, CubeVisual, MeshVisual, etc... 15:01 rubenwardy this is turning into an ECS 15:01 Unarelith not really 15:02 celeron55 certainly not 15:02 Unarelith rubenwardy, I made a basic ECS implementation here actually: https://github.com/Quent42340/ZeldaOOL 15:02 Unarelith well mine is more an EC than an ECS since my controllers doesn't hold a list of entities, but it's close 15:03 Unarelith I made an article about it three years ago, but it's in french: http://blog.gnidmoo.tk/2015/06/03/systeme-dentites-a-composants/ 15:04 Unarelith (and well, the structure has evolved since) 15:05 Unarelith but rubenwardy, I think the small scale changes like I did on GenericCAO could be applicable to a lot of things in the code 15:05 Krock merging... 15:06 Unarelith and it really improves things. before this change if you wanted to add a visual type, you'd have to add your ISceneNode and edit every piece of code in GenericCAO that uses visuals 15:07 Unarelith now you can just add one line in GenericCAO and implement IGenericCAOVisual 15:08 Unarelith wow 6 PRs merged today, you're on fire :p 15:08 Unarelith *7 15:09 celeron55 GenericCAO is kind of a patch on top of a system that had to start doing things it wasn't designed for 8) but sounds like good rework 15:10 Unarelith I noticed that actually, but it works well so it's not a problem, I just tried to make it more DRY/SRP/KISS 15:11 Unarelith and I'll probably do that on most of the code I guess 15:11 Krock Unarelith: didn't have the time yesterday 15:11 Krock and these are the simple PRs which don't stay open for a long time 15:12 Krock bugfix on the 2nd rank and features on 3rd.. they sometimes stall until they die in the depths of rebase 15:12 Unarelith Krock, btw I noticed old PRs which have probably be forgotten: https://gist.github.com/Quent42340/62ae591da5c75872908a8032113ae85a 15:13 Krock not forgotten. nobody keen to review 15:13 Unarelith some of them have probably been merged since (some are yours) 15:14 Unarelith if there's any way I can help to speed up 5.0.0 release, please tell me 15:14 Krock uhm.. you could share test results for all [Testing needed] and [High priority] PRs 15:14 Krock but that takes muuch time 15:14 ANAND Krock, since you've already reviewed #7557 extensively, would you be able to take a look at it again? 15:14 ShadowBot https://github.com/minetest/minetest/issues/7557 -- Expose player FOV to Lua API by ClobberXD 15:15 Krock ironically I'm not very keen on having this merged since it's a client setting 15:15 Krock it looked good the first time but well.. 15:16 ANAND This is just a generic alternative to the hard-coded zoom we have now 15:16 Krock generic but with lag 15:17 ANAND I agree it would be relatively laggier than the pure client-side zoom. Can it be improved in any way? 15:17 Unarelith Krock, I meant dev tasks, I can't make node-bound entities yet, but there's maybe other things I can work on 15:17 Krock which reminds me of trying out to interpolate FOV changes and improving the cinematic mode with PID 15:17 Krock reviewing PRs is also dev task 15:18 Krock but if you mean programming.. I don't know? maybe fixing a few bugs? 15:18 rubenwardy https://github.com/minetest/minetest/labels/Blocker 15:19 rubenwardy those all have counter part PRs though 15:19 Krock not sure how mainmenu prevents from gameplay but well 15:19 T4im go for the label Bug then 15:19 T4im in the issues that is 15:20 Krock heh. PRs which add bugs 15:20 T4im :D 15:20 Krock well, sometimes they do but having a label for them would be awkward 15:21 Unarelith fixing bugs in a code I'm not very familiar with will only add a layer of hacks on top of an hacky code 15:21 Unarelith adding things will give me refactoring ideas though (IGenericCAOVisual came from the fact that I needed to add a visual type to try my nodeentity thing) 15:22 T4im there is a dooming release; don't be surprised people prefer improving over enhancing at the moment 15:23 Unarelith enhancing will allow faster improving 15:23 T4im but not before the release 15:23 T4im enhancing might actually add new bugs without fixing anything itself 15:24 T4im simply a timing issue there 15:24 Unarelith "might", except I'm mainly taking about simple refactorings (ie. moving code somewhere else to split files) 15:25 Unarelith so no functional code change, would be pretty clumsy to add bugs by doing that 15:25 T4im but not unheard of :p 15:26 Unarelith definitely, but a proper git diff check, small commits and a good language comprehension should be enough to avoid that 15:34 p_gimeno um, missed an interesting discussion... 1) Unarelith, if you intend to keep KubKraft for yourself only and not share it with anyone at all, GitHub is not the best idea, but if you intend to let others see it and be able to try it, then a license is a MUST 15:35 p_gimeno 2) the fallacious mantra "Premature optimization is the root of all evil" is the root of all evil. http://ubiquity.acm.org/article.cfm?id=1513451 15:35 T4im :D 15:36 p_gimeno MT is notably affected by lack of optimization on critical parts, and it's getting harder and harder to tackle it 15:36 Unarelith p_gimeno, well, KubKraft is what I call a "learning project", which means I'm not trying to do a full featured game but learning things, I don't really care what people do with my code, so I didn't put a license on it 15:37 Unarelith no licence means I choose what people do with it, and I choose to not care 15:37 p_gimeno Unarelith: without a license, no one can (legally) put their hands on your code, not even download it - default copyright laws apply 15:37 rubenwardy no license means all rights reserved 15:37 p_gimeno ^ 15:37 rubenwardy sounds like you'd like a ultra permissive license like ISC or CC0 with a warranty disclaimer 15:38 Unarelith well this code isn't usable in a real project anyway, that's just my own personal training code that I release with anybody wants to read it 15:39 Unarelith 2) Premature optimization IS root of all evil, but some interpretations of that sentence may be wrong. 15:40 p_gimeno now, imagine I want to learn from it - am I supposed to learn from it by reading it only, or should I download it and try it in order to see the effects of my changes? if you intend the latter, then you're blocking it by not allowing a license 15:40 Unarelith and if MT lacks optimization it's because of its global structure 15:40 rubenwardy no 15:41 rubenwardy MT lacks optimization because people prefer to add features than to fix or optimise existing code 15:41 p_gimeno yeah, because why do premature optimization? someone will optimize it later :P 15:42 p_gimeno the design of the Lua vector type is an example of a serious flaw that has an effect on the code speed 15:42 Unarelith to get back on 1), https://softwareengineering.stackexchange.com/a/148165 you can legally do what you said in your example (personal use) and you can ask me directly for other uses, so seems less restrictive than what you said 15:43 Unarelith and for 2), optimizing an unfinished feature is useless, but once this feature is complete, you have to optimize it 15:43 Unarelith but optimizing at the very end of the development process is silly 15:44 T4im the abstract issue extends past that single mantra; you get yourself a junior programmer learning a new tool or mantra, and suddenly with that new hammer everything looks like a nail; (xml is so extendable! lets do imperative xml! yikes) the issue isn't that the mantra is wrong, the issue it's being overgeneralized 15:46 T4im language fanboyism is probably the most common manifestation of that 15:46 p_gimeno Unarelith: I don't think copying for "personal use" is allowed either, despite what that commenter says 15:47 p_gimeno Unarelith: I'd recommend reading the link in full about premature optimization 15:52 Unarelith p_gimeno, I really think it is allowed as long as the code in on GitHub (it's in the ToS) 15:54 Unarelith p_gimeno, and for the link you gave me, I couldn't agree more, but I still agree with the fact that a feature optimization is unnecessary until its implementation is over 15:54 T4im the personal use thing might be limited to jurisdiction specific exceptions 15:57 Unarelith anyway, people with a bit of common sense would guess that I won't sue them for unauthorized code usage 16:00 Unarelith and if one of my projects grows and becomes something interesting, then I'll add a license to open it to contributions, but until then I don't really care 16:00 T4im just checked, even within the eu the directive 2001/29/EC only permits member states to except private copies instead of enforcing it 16:01 p_gimeno github does require you to grant the right to copy it to other users: https://help.github.com/articles/github-terms-of-service/#5-license-grant-to-other-users but no other right, most notably not the right to modify it e.g. for private use or learning 16:01 ShadowBot https://github.com/minetest/minetest/issues/5 -- Fixed key names so the key set menu now works. by adamnew123456 16:02 rubenwardy I suggest taking this to #minetest 16:02 p_gimeno yes rubenwardy, sorry for the drift 16:06 Unarelith rubenwardy, Krock, btw, could you please take a look at my last PRs? the four of them needs discussion, so since we're all here it'll be easier to talk about it 16:09 Krock !tell ANAND Your FOV PR looks good apart from the code style comments which aren't resolved yet 16:09 ShadowBot Krock: O.K. 16:13 Krock I'm not keen on #7909. You assume the local clones are always in clean state; which doesn't have to be the case 16:13 ShadowBot https://github.com/minetest/minetest/issues/7909 -- CMakeLists: Use file(GLOB) instead of manually writing filenames by Quent42340 16:14 Unarelith what do you mean? 16:14 Krock rather find a way to unify the file lists so that the Android part does not need adjusting each time 16:15 Unarelith Android uses a Makefile, I can use a wildcard there too, that PR is an example and I can extend it to whatever 16:16 Unarelith the only side effect of file(GLOB) is having to do 'cmake && make' instead of 'make' each time (or only when a file is created/moved/deleted, but still better than manually writing all filenames) 16:16 T4im "only" 16:16 T4im that's hell if you are trying to git bisect 16:16 Krock surely it's easier to just include all files, but as you can see "settings_translation_file.cpp" is already the first file which doesn't match this pattern 16:17 Krock my build scripts are only set up for "make" 16:17 Unarelith Krock, settings_translation_file is a special case and should be handled like so 16:17 Unarelith and for build script, you just have to add "cmake &&" before "make" 16:17 Krock how long until the next special case appears? It's dangerous to include whatever there is 16:17 Unarelith use a folder in this case 16:18 Krock a directory for what? 16:18 Unarelith for example porting_android.cpp needs its own folder 16:18 Unarelith file(GLOB) will not look for files recursively, so a folder will hide some files to it 16:18 T4im you can't enforce people to run cmake every pull either; so things will break, and people will come here and ask why 16:19 Krock ' 16:19 Krock ^ * 16:19 T4im much rather spend some time listing files explicitly instead of having to spend a lot more time explaining and fixing and attending to all kinds of subtle problems that will appear 16:20 Krock I see that it's very easy to just put in some new files, it works, yay, great. But side-effects like the above arise which are more annoying than just adding another new line to the cmake config 16:20 Unarelith *adding another new line to the cmake config + the Android makefile + sometimes clang-format-whitelist.txt 16:21 Unarelith and when you rename/move multiple files it's a real pain 16:21 Krock there's no need for clang if you format it properly and use // clang-format off switches where it's not possible in another way 16:22 Unarelith it's still two files to edit 16:22 Krock a pain each 4 years is probably acceptable 16:22 T4im this is again a case where you are hammering the nail with the wrong tool; dry except repeat yourself where necessary 16:22 Krock hence "try to unify android makefile with cmake" 16:22 T4im that would be good :D 16:23 Krock it would be great, but yet no PR for it (or is there?) 16:24 Unarelith Krock, it's possible to write post-pull git hook to do the "cmake" automatically 16:24 T4im and it's not possible to distribute that hook to everyone automatically 16:24 Krock everything is possible but telling it to a crowd of more-or-less random people 16:25 T4im understand that noone likes to adapt cmake files to add sources; the fact that everyone is doing it comes from it being the lesser evil, not them never having heard of globbing 16:26 Unarelith then they should learn 16:26 Krock would be a great world if everything were that simple 16:26 Unarelith everything isn't that complicated, really 16:27 T4im but seriously, how do you deal with git bisect in the project you use globbing with, Unarelith? 16:27 Unarelith but I think a basic documentation of MT codebase could be useful to contributors 16:28 Unarelith T4im, I don't see the problem with git bisect 16:29 T4im well, do you really expect someone bisecting to run a reconfiguration every step? 16:30 T4im or how about switching back and forth between branches 16:31 Unarelith ok just use something like "alias make='cmake && make'" if you're so unhappy about writing a few more characters 16:31 T4im you keep thinking like you are the only person in the project 16:32 T4im you cannot distribute aliases either 16:32 Unarelith because when you switch between branch, or when you're switching back to an old commit, sometimes the build system changed, so you still have to reconfigure 16:32 T4im and even if you could, it would be mad running cmake every time 16:32 T4im unnecessarily 16:32 Unarelith why would it be? 16:32 Unarelith it runs in less than a second 16:35 T4im well; i can't actually come up with a killer argument against it; but it would raise the question why cmake caches in the first place (because you are emptying that cache every single time, so it becomes redundant) 16:37 Unarelith T4im, this could work: https://stackoverflow.com/questions/17832459/getting-cmake-to-run-before-building-after-pulling-from-git/17838951#17838951 16:38 T4im drawbacks mentions stashes 16:38 rubenwardy Still doesn't help as people would need to install the hook 16:38 rubenwardy I'm -1 on that PR also 16:39 T4im i suspect you could just write a complicated script that you run every time to check the entire file tree; but you're starting to walk towards a complicated hack solution in order to use a KISS thing; when you should really KISS and not do any of that 16:40 T4im wait until a newer cmake is used and then use the cmake provided solution for that problem 16:40 T4im until then keep it simple 16:43 T4im heck, or write a util script that generates the individual src-file listings for you on request 16:49 Unarelith a more hacky but simpler solution would be to write a wrapper Makefile to run cmake each time 16:50 T4im but why.. if you have a choice between running cmake each time and not, why chose to do it? if you want to avoid adding files to the cmakelists, create a script to do it for you 16:50 Krock Current stats: 1 vs 3. 17:00 Unarelith ok so I managed to make it work your way: https://pastebin.com/xWUPchKW 17:01 Unarelith if I add this, cmake is automatically run everytime the user runs "make" 17:01 Unarelith (sorry my sentence made no sense) 17:02 Unarelith is it ok for you Krock, rubenwardy, T4im? 17:09 rubenwardy I still don't like it 17:09 Unarelith ok, I close this PR then 17:12 Unarelith rubenwardy, Krock, there's still file naming issues for #7903 and #7908, do you prefer PascalCase (like class names and what I used here), minetestcase or snake_case? 17:12 ShadowBot https://github.com/minetest/minetest/issues/7903 -- File 'content_sao' splitted into folder 'src/server/object'. by Quent42340 17:12 ShadowBot https://github.com/minetest/minetest/issues/7908 -- File 'client/game' splitted into new folder by Quent42340 17:14 Krock minetestcase also includes snake_case partially since it's somewhat inconsistent 17:14 Unarelith I didn't have a name for this 17:15 Krock ikr. It's all a bit mixed 17:17 Krock if the files are dedicated to one class, then I think it's OK to have the files named the same way. 17:19 Unarelith there's the NodeDrawType and co. issue though 17:20 Unarelith ContentParamType, ContentParamType2, LiquidType, PlantlikeStyle and NodeDrawType should be in the same file imo 17:22 Unarelith but how should that file be named? accordingly to the main unit name, it would be "NodeDrawType.h" (since others are smaller and somewhat related), but it doesn't make sense 17:31 Krock drawtypes.h 17:32 Krock contenttypes.h 17:33 Unarelith oh, yep, would work 17:33 Unarelith but draw_types.h or DrawTypes.h would be better 17:39 VanessaE Krock: got a moment? 17:41 Krock yep 17:41 VanessaE Krock: https://github.com/minetest/minetest/pull/7916#pullrequestreview-180563554 if I "Commit suggestion", I presume it'll just work, but I feel like I'm just not understanding.. I barely understand my own changes :) 17:42 Krock it won't work the way it should because it only changes one line instead of three 17:43 Krock you're doing it fine. adapt the suggested change and remove if (i < PLAYER_INVENTORY_SIZE && i < player->hud_hotbar_itemcount) { below 17:43 Krock because "i" will always be within the correct range 17:44 VanessaE OHHHHH 17:44 Krock unless this was added on purpose if it's multithreaded.. shouldn't be a problem I think 17:45 VanessaE like this? 17:45 VanessaE (new commit) 17:45 VanessaE https://github.com/minetest/minetest/pull/7916/commits/93eba0620ad3ca1e987c069bc566a0d30c631d45 17:46 Krock heh. what about the indents? 17:46 VanessaE hmm, need to unintend a bit 17:46 Krock will test 17:47 VanessaE force-pushed to fix the indent 17:48 Krock oh. it should be "i <= max_item;". If you look at its definition, it's pointing to the last slot index 17:48 VanessaE oops, I missed thws 17:48 VanessaE that* 17:48 VanessaE force-pushed 17:59 Krock 200 18:00 VanessaE ? 18:00 Krock OK. approved 18:00 VanessaE oh, http 200 :) 18:00 VanessaE \o/ 18:01 VanessaE btw, technically two approvals, if paramat's comment counts 18:03 rubenwardy the comment doesn't account 18:03 rubenwardy -ac 18:03 VanessaE ok 18:04 VanessaE thanks for helping me work that out, Krock 18:05 Krock yw :) 18:13 VanessaE rubenwardy: aren't you the least bit surprised that I (of all people) had the intestinal fortitude to try to do something useful in the engine? :) 18:13 rubenwardy I'm not sure intestinal is the word you mean 18:14 VanessaE the guts :P 18:17 Krock just put it into the translator and chuckled a bit 19:42 p_gimeno rubenwardy: are you sure Irrlicht uses Y for up instead of Z? as opposed to just Minetest's usage of Irrlicht's axes? 19:49 rubenwardy I'm fairly sure, I used irrlicht for other projects before finding Mt 19:49 rubenwardy I actually found Mt on the irrlicht forums 19:51 rubenwardy Y being up is a direct X thing 19:51 p_gimeno I see, yuck 19:52 rubenwardy I had the same reaction when I learned that maths used z as up 19:53 Krock ^ can feel this as well 19:57 p_gimeno it's hard to bite my tongue, but that discussion is too off-topic here as it's an Irrlicht thing and MT just seems to have inherited it 22:12 paramat "if the files are dedicated to one class, then I think it's OK to have the files named the same way." any more thoughts on PascalCase versus snake_case for single unit files? we should discuss soon due to those 2 organisation PRs. i'm feeling somewhat neutral on this. rubenwardy 22:26 paramat KISS is actually a central philosophy of MT, and i tend to push for that harder than anyone, but oddly get resistance and become unpopular for it =) there's a lot of desire amongst users for overly fancy features 22:29 paramat Unarelith your enthusiasm, talent and help is very much appreciated. it's unfortunate that your free time co-incides with us desperately trying to get a very big release out while also having little core dev time. so in some ways it's the worst time to make big changes to MT (although these file moves are not a problem) 22:29 sfan5 merging 22:29 sfan5 #7916 in 5 minutes 22:29 ShadowBot https://github.com/minetest/minetest/issues/7916 -- raise hotbar limit to 32 slots, add associated keybinding options by VanessaE 22:29 VanessaE \o/ 22:29 VanessaE you want me to squash that first? 22:30 sfan5 i can do that on merge 22:31 VanessaE ok. 22:34 Unarelith thanks paramat, and yes I noticed that it's not the good time :/ 22:45 Unarelith rubenwardy, "m_ is rarely used in Java (and is discouraged in future code in our code base)" <= really? :O 22:47 paramat sfan5 PascalCase or snake_case for single unit files? 22:49 paramat PascalCase: Krock is ok with it, i'm neutral, i seem to remember rubenwardy is ok with it (please confirm) so your PRs are probably ok as they are 22:49 sfan5 the existing files are snake case, so I'd keep that 22:49 paramat lol 22:50 Unarelith some of them are minetestcase, some of them are snake_case, why not add PascalCase :p 22:57 paramat Unarelith what we certainly need is PR reviews (especially on the 'blockers'), although you're not a core dev your reviews will be useful and appreciated. testing is also very useful 23:00 Unarelith paramat, unfortunately 2 on 3 of the blocker PRs are coredev PRs, the other one looks good but it'll need android testing 23:09 Unarelith actually paramat, #7891 is ready I guess, that would close 7447 too 23:09 ShadowBot https://github.com/minetest/minetest/issues/7891 -- Fix ContentDB packages timing out by using download_file instead by rubenwardy 23:15 paramat down to 100 PRs at last 23:19 paramat wil merge #7601 in a few mins 23:19 ShadowBot https://github.com/minetest/minetest/issues/7601 -- Added Table of Contents by woshiicyrus 23:21 paramat will also merge #7922 23:21 ShadowBot https://github.com/minetest/minetest/issues/7922 -- Improve world configure menu by pauloue 23:26 paramat and #7820 woohoo 23:26 ShadowBot https://github.com/minetest/minetest/issues/7820 -- Update Android java-part by MoNTE48 23:26 Unarelith poor #7891 :'( 23:26 ShadowBot https://github.com/minetest/minetest/issues/7891 -- Fix ContentDB packages timing out by using download_file instead by rubenwardy 23:28 paramat that's likely fine will probably be merged soon 23:28 paramat merging those 3 now ... 23:44 paramat all merged