| 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:48 |
|
basxto joined #minetest-dev |
| 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 |
| 03:34 |
|
Foz joined #minetest-dev |
| 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 |
| 05:01 |
|
reductum joined #minetest-dev |
| 05:05 |
|
sys4 joined #minetest-dev |
| 06:17 |
|
fireglow joined #minetest-dev |
| 06:26 |
|
fireglow joined #minetest-dev |
| 06:41 |
|
Cornelia joined #minetest-dev |
| 06:59 |
|
paramat joined #minetest-dev |
| 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 |
| 07:46 |
|
Beton joined #minetest-dev |
| 08:38 |
|
Gael-de-Sailly joined #minetest-dev |
| 08:43 |
|
p_gimeno joined #minetest-dev |
| 08:51 |
|
Wuzzy joined #minetest-dev |
| 09:23 |
|
Krock joined #minetest-dev |
| 10:10 |
|
Mensious joined #minetest-dev |
| 10:46 |
|
Fixer joined #minetest-dev |
| 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:18 |
|
calcul0n joined #minetest-dev |
| 12:33 |
|
YuGiOhJCJ joined #minetest-dev |
| 12:42 |
p_gimeno |
wasn't DIG called PUNCH? |
| 12:43 |
ANAND |
AFAIK, it was just called LMB |
| 12:44 |
ANAND |
is* |
| 12:45 |
|
jas_ joined #minetest-dev |
| 12:47 |
nerzhul |
generic way can be nice |
| 12:47 |
nerzhul |
:) |
| 12:52 |
|
calcul0n joined #minetest-dev |
| 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:07 |
|
fwhcat joined #minetest-dev |
| 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 |
<Unarelith> 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 |
<Krock> 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:32 |
|
Gael-de-Sailly joined #minetest-dev |
| 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:04 |
|
Ruslan1 joined #minetest-dev |
| 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 |
|
behalebabo joined #minetest-dev |
| 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 |
|
pauloue joined #minetest-dev |
| 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 |
| 18:53 |
|
Xio joined #minetest-dev |
| 19:38 |
|
Taoki joined #minetest-dev |
| 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 |
|
proller joined #minetest-dev |
| 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 |
| 20:31 |
|
reductum joined #minetest-dev |
| 21:43 |
|
jas_ joined #minetest-dev |
| 22:09 |
|
paramat joined #minetest-dev |
| 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:00 |
|
troller joined #minetest-dev |
| 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 |
