| Time |
Nick |
Message |
| 00:26 |
|
Unarelith joined #minetest-dev |
| 00:40 |
|
Gael-de-Sailly joined #minetest-dev |
| 00:42 |
p_gimeno |
does any core developer have the resources for the cross OS verification of #7768 requested by nerzhul in <https://github.com/minetest/minetest/issues/5521#issuecomment-440451335>? |
| 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:47 |
|
ANAND joined #minetest-dev |
| 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 |
| 02:45 |
|
Ruslan1 joined #minetest-dev |
| 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 |
| 04:55 |
|
Cornelia joined #minetest-dev |
| 05:05 |
|
ssieb joined #minetest-dev |
| 05:07 |
|
DI3HARD139 joined #minetest-dev |
| 05:21 |
|
Lia joined #minetest-dev |
| 06:00 |
|
jas_ joined #minetest-dev |
| 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 |
| 09:13 |
|
jas_ joined #minetest-dev |
| 10:24 |
|
Gael-de-Sailly joined #minetest-dev |
| 10:24 |
|
proller joined #minetest-dev |
| 11:14 |
|
proller joined #minetest-dev |
| 11:28 |
|
Fixer joined #minetest-dev |
| 11:37 |
|
Mensious joined #minetest-dev |
| 11:52 |
|
Mensious joined #minetest-dev |
| 12:02 |
|
troller joined #minetest-dev |
| 12:09 |
|
proller joined #minetest-dev |
| 12:33 |
|
jas_ joined #minetest-dev |
| 12:36 |
|
proller joined #minetest-dev |
| 13:05 |
|
calcul0n joined #minetest-dev |
| 13:13 |
|
Gael-de-Sailly joined #minetest-dev |
| 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:45 |
|
Wuzzy joined #minetest-dev |
| 13:46 |
Unarelith |
just go and ask ##c++-generaln you'll see |
| 13:46 |
Unarelith |
*##c++-general |
| 13:51 |
|
proller joined #minetest-dev |
| 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` |
| 14:20 |
|
twoelk joined #minetest-dev |
| 15:40 |
|
longerstaff13-m joined #minetest-dev |
| 16:01 |
|
TC01 joined #minetest-dev |
| 16:09 |
|
YuGiOhJCJ joined #minetest-dev |
| 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<Class>(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 |
| 16:46 |
|
troller joined #minetest-dev |
| 17:08 |
|
twoelk left #minetest-dev |
| 17:15 |
|
Mensious joined #minetest-dev |
| 17:16 |
|
proller joined #minetest-dev |
| 17:39 |
|
Krock joined #minetest-dev |
| 17:48 |
|
sys4 joined #minetest-dev |
| 17:50 |
Unarelith |
<rubenwardy> 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 |
|
DI3HARD139 joined #minetest-dev |
| 17:53 |
Unarelith |
<nerzhul> 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:04 |
|
Icedream joined #minetest-dev |
| 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:09 |
|
proller joined #minetest-dev |
| 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:34 |
|
ssieb joined #minetest-dev |
| 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:52 |
|
proller joined #minetest-dev |
| 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:09 |
|
Krock joined #minetest-dev |
| 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:26 |
|
sys4 joined #minetest-dev |
| 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 |
|
proller joined #minetest-dev |
| 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 |
| 20:15 |
|
proller joined #minetest-dev |
| 20:21 |
|
Cornelia joined #minetest-dev |
| 20:37 |
|
Cornelia joined #minetest-dev |
| 21:40 |
|
proller joined #minetest-dev |
| 21:43 |
|
Ruslan1 joined #minetest-dev |
| 21:53 |
|
sys4 joined #minetest-dev |
| 22:59 |
|
Cornelia joined #minetest-dev |
| 23:25 |
|
Megaf_ joined #minetest-dev |
| 23:28 |
|
Calinou joined #minetest-dev |
| 23:46 |
|
YuGiOhJCJ joined #minetest-dev |
