Time Nick Message 00:42 p_gimeno does any core developer have the resources for the cross OS verification of #7768 requested by nerzhul in ? 00:42 ShadowBot https://github.com/minetest/minetest/issues/7768 -- Network: Send IEEE floats by SmallJoker 00:42 Unarelith paramat, the PascalCase for filenames is because each file defines one unit, other files can keep old convention because they define multiple units, do you really want me to change this? 00:57 paramat well let's all discuss first 00:58 paramat 'defines one unit' ok i see, it makes sense in a way, but doesn't change the readability issue 00:59 Unarelith then all classes in the code have readability issues? 01:00 paramat that's different 01:01 Unarelith for me it's the same exact thing, but well, it's another snake_case vs camelCase/PascalCase debate, so I'll just change names, desperately 01:01 paramat it's very helpful when coding to be able to scan through a directory and have clear readable filenames, especially now we will have many more. but let's see what other devs think 01:03 paramat we can always discuss dev rules, but as long as it's an official dev rule it has to be followed in a PR :) 01:04 Unarelith people seemed to be ok with it when they reviewed #7903, the only problem was .hpp -> .h 01:04 ShadowBot https://github.com/minetest/minetest/issues/7903 -- File 'content_sao' splitted into folder 'src/server/object'. by Quent42340 01:05 Unarelith btw paramat, I don't know if it can be useful but I made this: https://gist.github.com/Quent42340/62ae591da5c75872908a8032113ae85a 01:06 paramat maybe wait a day or so before making any change 01:06 Unarelith ok 01:08 paramat hmm well ... what is 'important' seems subjective, and we do have a 'high priority' label already. also, all PRs are 'possible merge' :) 01:12 paramat however, bringing attention to PRs is welcome 01:13 Unarelith I searched for old, mergeable and small PRs and yeah, I could have named my titles better 01:15 p_gimeno I'd also consider #6898 which is closed in GitHub but open in NotABug (kind of pending on #7901) 01:15 ShadowBot p_gimeno: Error: Delimiter not found in "Connection timed out." 01:15 ShadowBot https://github.com/minetest/minetest/issues/7901 -- Accepting PRs/patches from non-Github users 01:22 p_gimeno a future proofing change, if that one is not accepted for 5.0.0 due to lack of developer time, is to change the check for is_yes() in load_mod_xxxx to checking for the string "false" 01:23 p_gimeno that would allow adding that PR to 5.x.y later 01:37 p_gimeno I mean something like this: https://paste.scratchbook.ch/view/45da9e55 plus the equivalent on the Lua side, which I assume would be much easier to review and merge 01:40 p_gimeno I can write a more complete patch, but someone else needs to turn it into a PR 03:30 Unarelith ... guys, did you know that C++ keywords `new` and `delete` should never be used since C++11? 03:30 Unarelith the code is full of them 03:32 Unarelith There's also a lot of `NULL` instead of `nullptr`, a lot of `virtual ...` instead of `... override`, and missing `override` too, usage of constructor initialisation instead of in-class definition, `typedef b a` used instead of `using a = b`, the list goes on and on 03:57 sofar maybe use coccinelle to fix them up 03:58 sofar oh, no, it only does C atm 04:04 Unarelith afaik clang-tidy can be configured to check this kind of stuff 04:14 sofar yeah, probably 04:14 sofar although it can really mess up stuff, too :D 04:15 Unarelith you mean clang-tidy? 04:16 Unarelith it has a read-only mode 08:08 nerzhul Unarelith, 5.0.0 is the first C++11 compatible version, we don't rewrite the whole code. Also what did you use if you don't use them ? 08:08 nerzhul unique_ptr ? shared_ptr ? 08:08 nerzhul we don't tend to use them more than expected 08:10 nerzhul also new and delete are not deprecated keywords :) neither in C++17 13:44 Unarelith nerzhul, they're not deprecated, but since C++14's make_unique we have no use for them anymore 13:44 Unarelith And yes I use unique_ptr/shared_ptr instead, way better 13:46 Unarelith just go and ask ##c++-generaln you'll see 13:46 Unarelith *##c++-general 13:55 sfan5 Minetest's codebase originally was c++03, so it's not surprising that there's lots of "legacy" C++ features being used 13:55 sfan5 also note that we currently support (minimum) c++11, so std::make_unique is out of question 13:55 Unarelith you can still provide it, 13:56 Unarelith but it's not really needed since it's just a wrapper around new 13:56 sfan5 yeah 14:00 Unarelith and legacy code may be ok, but things like `extern Clouds *g_menuclouds;` are not legacy code, it's bad practice to dynamically allocate globals like `g_menuclouds` 16:35 nerzhul we use c++11 not 14 16:35 nerzhul and no we don't intend to refactor the whole code to use shared_ptr unique_ptr 16:35 nerzhul there is architectural technical debt in many places, it's more a source of risk than a useful thing 16:36 nerzhul also in c++-general you have only devs who wants to code less :D 16:36 nerzhul mt code architecture is not ready for those containers in old code 16:36 nerzhul many in new code but not old 16:37 nerzhul also, it's important to know the object lifetime, and shared_ptr hide that 16:37 rubenwardy unique_ptr does indicate it well for that case 16:37 rubenwardy the case being something owned by one owner 16:38 nerzhul yes but you cannot modify outside, and we have many many code modified from many many places 16:38 rubenwardy like, "Client" owned by "Game" 16:38 nerzhul :p 16:38 nerzhul yes but singleton instance are easy to manage 16:38 nerzhul now use mapblock 16:38 rubenwardy and from unique_ptr it's typically to get a raw pointer 16:38 nerzhul with a shared_ptr :p 16:38 rubenwardy ew 16:38 nerzhul not exactly because you can not copy it 16:39 nerzhul (properly) 16:39 rubenwardy surely to copy you'd do std::unique_ptr(new Class(old)); 16:39 rubenwardy ie: the copy constructor 16:39 rubenwardy also 16:39 rubenwardy I don't think it's worth being anal with smart pointers at this point 16:40 nerzhul yes but i mean pass unique_ptr to anther object :)à 16:40 rubenwardy you shouldn't pass unique_ptr to another object 16:41 nerzhul i'm fine with shared_ptr and unique_ptr for new code, not old 16:41 rubenwardy I'll be using shared pointers when I get around to implementing PlayerMetadata when offline, simply because it makes sense 16:41 nerzhul yeah but MT code has too many technical debt then we cannot use them properly heh 16:41 rubenwardy essentially, yeah 16:41 nerzhul you mean your data is always in memory ? 16:41 nerzhul why ? 16:41 rubenwardy no 16:42 rubenwardy it'll load the meta on demand, and use a shared pointer to allow multiple references to it 16:42 nerzhul and leak ? 16:42 nerzhul you code allow to load the same metadata multiple times ? 16:42 rubenwardy no 16:42 rubenwardy only once 16:42 nerzhul then why not use unique_ptr and weak_ptr ? 16:42 rubenwardy and there would be a leak if mods keep a reference to it 16:43 nerzhul i think you have only a registration map no ? 16:43 nerzhul okay the risk is mod have an obsolete pointer ? 16:44 rubenwardy the point of shared pointer is to avoid having multiple copies of the same metadata, and ending up with inconsistent data 16:44 rubenwardy and also to allow Minetest to know how many references there are to it 16:44 nerzhul like any pointer 16:44 nerzhul except refcount 16:45 nerzhul the only real usecase of a share_ptr is don't know when the object is unused 16:45 nerzhul and it's only in rare cases you don't know when it's removed 17:50 Unarelith you shouldn't pass unique_ptr to another object <= You can still move it (since you can't copy it) if you want to transfer ownership 17:51 Unarelith unique_ptr and shared_ptr usage should make lifetime really clear: who owns the objects deletes it 17:51 p_gimeno wasn't that auto_ptr? 17:52 Unarelith p_gimeno, yep, c++03 auto_ptr => c++11 unique_ptr and shared_ptr 17:53 Unarelith then why not use unique_ptr and weak_ptr ? <= because weak_ptr is tied to a shared_ptr, not to an unique_ptr iirc 18:05 Unarelith Krock, I want to do node-bound entities now, but I'll need your help, since you're "the entity attachment guy" 18:12 Unarelith first, can entity attachment be used to develop this feature? or should I make a new node type? 18:13 Unarelith oh, and are attached entity removed on node removal? 18:45 nerzhul Unarelith your use case seems to be complicated for engine :p 18:45 nerzhul node is not an entity 18:45 nerzhul it's two non related objects 18:46 Unarelith nerzhul, can't you attach entities to nodes? or maybe it's a visual feature? 18:55 T4im you can attach entities to other entities; like a player to a boat or a horse, so that the childs position remains relative to the parent and can move with it 18:57 Unarelith oh I see 19:10 Krock Unarelith: attachments are limited to entities 19:10 Krock but it depends on irrlicht whether we can also attach to nodes 19:10 Unarelith why does it depend on irrlicht? 19:11 Krock because irrlicht handles the positions of the models 19:11 Krock i.e. everything what's drawn 19:13 Unarelith yep, I know that, but I don't see the relation between that and entity attachment 19:13 Krock well, attachments also have visual parts, and irrlicht also offers attaching multiple SceneNodes together 19:14 Krock so we let irrlicht handle the drawing attachments, but also have to mirror that for server-side use to determine where the objects are 19:14 Krock *are attached to 19:14 Unarelith and that part works between entities? 19:15 Krock yes it does. however, collision detection yet does not support attachments for the simple reason that it's not implemented yet 19:16 Unarelith collision detection wouldn't be a problem since these nodeentities would be mainly used for animation purpose 19:16 Krock however, the fact that Minetest is a multiplayer game lets it break many times when players are attached to something or vice-versa 19:17 Krock the attachment system is kind of luck whether it works or not. ugly workarounds are sometimes also needed so that newly joined players also see it properly 19:18 p_gimeno incidentally, Second Life doesn't support collisions with attachments either :) 19:20 Unarelith Krock, I assume Nodes aren't scene nodes? 19:21 Krock dunno. Total noob at rendering stuff 19:21 Krock could look it up tho 19:22 Krock mapblock_mesh.cpp .. 404 19:22 Unarelith ok makes sense, they're not scene nodes (would have been weird anyway) 19:23 Unarelith so making a nodeentity would require creating a new type of entity, not a new type of node 19:24 Unarelith UnitSAO and LuaEntitySAO represents which objects exactly? 19:25 Krock LuaEntitySAO is basically minetest.add_entity (or spawn_entity) 19:25 Krock and all entity functions around that 19:25 Krock UnitSAO .. isn't LuaEntitySAO inherited from that? 19:26 Unarelith it is, but I was wondering if it was itself used 19:26 Krock so that would include all server active objects, based on active objects 19:26 Krock it's only used as base for other classes 19:27 Krock TestSAO is probably only used in the unittests, if at all. unused in-game 19:27 Krock for the client, everything is a GenericCAO anyway 19:30 Krock testing #7817 19:30 ShadowBot https://github.com/minetest/minetest/issues/7817 -- Add pitch fly mode by Gael-de-Sailly 19:35 Unarelith Krock, do you know the SRP? 19:36 Krock refer to est31 19:36 Unarelith est31? 19:36 Krock yes. former core dev 19:36 Krock they're still online on freenode 19:40 Unarelith nvm, I'm wondering can nodeboxes be used for entities? I guess not 19:40 Unarelith I don't know how they're drawn but for nodeentities nodeboxes will be reaaaaally useful 19:41 Krock they can. again using the wieldmesh type 19:41 Krock look into how "wieldmesh" is generated for entities. there it should be