Time |
Nick |
Message |
00:56 |
|
crazyR joined #minetest-dev |
01:05 |
|
est31 joined #minetest-dev |
01:06 |
est31 |
cheapie, can you test around a bit and perhaps make a pr? |
01:06 |
est31 |
it will slow down entering stuff into the db |
01:06 |
est31 |
so sort of a trade-off here |
01:11 |
est31 |
hrmm |
01:11 |
est31 |
is rollback enabled by default? |
01:12 |
est31 |
perhaps this could be a reason to change the default to disabled |
01:21 |
|
cib0 joined #minetest-dev |
02:50 |
|
cib0 joined #minetest-dev |
03:15 |
|
leat joined #minetest-dev |
03:16 |
|
crazyR joined #minetest-dev |
03:24 |
|
leat joined #minetest-dev |
03:50 |
hmmmm |
est, https://github.com/minetest/minetest/pull/3281 ?? |
04:59 |
|
cib0 joined #minetest-dev |
05:08 |
|
Miner_48er joined #minetest-dev |
05:39 |
|
est31 joined #minetest-dev |
05:39 |
est31 |
hmmmm, seen my comments on your PR? |
05:45 |
est31 |
also I don't see why its neccessary to remove the cleanup method |
05:45 |
est31 |
and integrate it into the places it gets invoked |
05:47 |
est31 |
but except for my comments the PR looks good |
05:49 |
est31 |
(I'm ok with removing the cleanup method, thats more of a nitpick) |
06:01 |
hmmmm |
the cleanup method was wrong |
06:01 |
hmmmm |
resources need to be freed after the first join() |
06:05 |
est31 |
okay then |
06:14 |
est31 |
well, then the only remaining point is about that m_joinable variable |
06:15 |
est31 |
in the current way it is used, it doesn't seem to be needed |
06:15 |
|
leat joined #minetest-dev |
06:15 |
est31 |
the mutex already takes care of two threads trying to join simultaneously |
06:15 |
est31 |
and m_running takes care of join being called multiple times |
06:16 |
est31 |
so what you do, to remove the m_running check, and add a m_joinable check isn't neccessary |
06:16 |
est31 |
and makes the class more complicated than needed |
06:16 |
est31 |
we can have m_joinable once we have compare and swap |
06:17 |
est31 |
then we can remove the mutex as well |
06:17 |
est31 |
but until then I dont think it should be there. |
06:19 |
est31 |
if that's fixed, I think we can merge the PR. If it still doesn't make it build for Krock he can make a PR. |
06:20 |
est31 |
and the doc comment is only a nitpick |
06:24 |
hmmmm |
again: m_joinable is NOT for that purpose |
06:25 |
est31 |
what is "that" purpose? |
06:25 |
hmmmm |
m_running == false does NOT imply m_joinable == false |
06:25 |
hmmmm |
did you read the unit tests? |
06:25 |
hmmmm |
the thread can exit on its own |
06:25 |
est31 |
no |
06:25 |
hmmmm |
m_running == false but m_joinable == true |
06:25 |
est31 |
ah |
06:25 |
hmmmm |
you still need to join that thread in order to free the resources |
06:25 |
hmmmm |
in addition... |
06:25 |
hmmmm |
m_running == false does not mean the thread is not still running |
06:26 |
est31 |
why do you need to join a thread in order to free its resources? |
06:26 |
hmmmm |
it means that it's not in its main run loop any longer, and that it may be starting up or shutting down |
06:26 |
hmmmm |
because that's the way pthread works |
06:26 |
est31 |
why cant you simply you know do delete m_thread |
06:26 |
est31 |
or so |
06:27 |
est31 |
well, then I approve |
06:27 |
est31 |
+1 |
06:28 |
|
Hunterz joined #minetest-dev |
06:33 |
est31 |
After you've merged it can you have a look at #3286 and https://github.com/est31/minetest/tree/rollback_proper_flush ? |
06:33 |
ShadowBot |
https://github.com/minetest/minetest/issues/3286 -- Improve rollback database indexing by cheapie |
06:33 |
hmmmm |
hmm |
06:33 |
hmmmm |
rollback is only sqlite so this seems okay |
06:34 |
hmmmm |
wow great :-) simple yet highly effective optimization |
06:34 |
hmmmm |
3286 has a +1 for me |
06:35 |
hmmmm |
86144d4 also looks very reasonable to me, +1 too |
06:36 |
est31 |
okay pushing both |
06:36 |
hmmmm |
make sure you check to see if your commit fixes any issues filed |
06:36 |
hmmmm |
this sounds like a combo bugfix and enhancement |
06:37 |
est31 |
for 3286 there is #1613 |
06:37 |
ShadowBot |
https://github.com/minetest/minetest/issues/1613 -- /rollback and /rollback_check are horribly slow |
06:38 |
est31 |
but I won't close it just jet |
06:38 |
est31 |
yet* |
06:38 |
hmmmm |
that sounds reasonable, but iqualfragile seems to be inactive for a while now |
06:38 |
hmmmm |
if I were you I'd close it with a "feel free to reopen if this is still an issue" |
06:39 |
est31 |
okay as well |
06:39 |
hmmmm |
we don't want to forget that 1613 has been addressed |
06:39 |
hmmmm |
I think the main problem is that RollbackManager logs too much stuff to be honest, that's by design |
06:40 |
est31 |
otherwise I can't find an issue for my commit: https://github.com/minetest/minetest/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+rollback |
06:40 |
est31 |
yeh |
06:40 |
est31 |
yeah* |
06:40 |
hmmmm |
it might be worthwhile to have a feature where server admins can select specific things to roll back |
06:40 |
hmmmm |
? |
06:40 |
est31 |
? |
06:41 |
hmmmm |
I don't know just thinking out loud |
06:41 |
est31 |
we could perhaps add a rollbackthread, and isolate the computation into queues |
06:41 |
hmmmm |
ehhh |
06:41 |
hmmmm |
the problem is the volume |
06:41 |
est31 |
then the server doesn't stop when sb does rollback computation |
06:41 |
est31 |
and checks |
06:42 |
hmmmm |
sb? |
06:42 |
est31 |
somebody |
06:42 |
hmmmm |
ah |
06:42 |
hmmmm |
a rollback thread would be a helpful first step I think |
06:42 |
est31 |
its not really interwined with other code, all the places it communicates you can queue-ify |
06:42 |
hmmmm |
because like I said, rollback is primarily I/O bound |
06:42 |
est31 |
yup |
06:42 |
hmmmm |
it doesn't make sense to me to block on posting rollback events |
06:43 |
hmmmm |
or wait |
06:43 |
hmmmm |
this needs to be looked at first, maybe it only flushes to DB every X number of events which somewhat mitigates this |
06:43 |
est31 |
yes |
06:43 |
est31 |
that is precisely what happens |
06:43 |
est31 |
all 500 events |
06:43 |
hmmmm |
oh |
06:43 |
hmmmm |
not sure if it's really big enough to make a new thread for then |
06:44 |
hmmmm |
adding a new thread is sort of a big deal imho |
06:44 |
est31 |
and guess what, when the server shuts down, it forgets the last elements in the queue |
06:44 |
est31 |
thats what my commit is about |
06:44 |
est31 |
partl |
06:44 |
est31 |
y |
06:47 |
est31 |
wtf |
06:47 |
est31 |
github is weird |
06:47 |
est31 |
it writes "cheapie committed for est31" |
06:50 |
hmmmm |
they must have just changed that bit of text today |
06:51 |
hmmmm |
it doesn't make as much sense as the previous sentence |
06:56 |
Hunterz |
est31: I tested yours ncurses console, looks like works fine, but after some time (idk hours, days) server runs, but when type any command to console server does not respond to commands |
06:58 |
est31 |
okay |
06:58 |
est31 |
can you still type |
06:58 |
Hunterz |
yes |
06:59 |
est31 |
so that when you press "a", it shows you "a" on the console? |
06:59 |
Hunterz |
server is now without console, but when I remember, not show me what I typed |
07:00 |
est31 |
ok |
07:00 |
Hunterz |
after press enter |
07:00 |
est31 |
you sure you haven't pressed ctrl + s inside the console window? |
07:01 |
Hunterz |
running mt in the screen, when leave pressing ctr +a then s |
07:03 |
Hunterz |
sorry ctrl +a then d |
07:04 |
est31 |
hrmm |
07:04 |
est31 |
the server is still responsive, so you still can log in via normal client? |
07:05 |
Hunterz |
yes players can play |
07:06 |
Hunterz |
now srtared server with --terminal I will report you soon when problem come again |
07:06 |
Hunterz |
started |
07:10 |
Hunterz |
when every line starts with <1> - this is some kinf of ncurses format ? |
07:12 |
est31 |
no |
07:12 |
est31 |
its the log level |
07:12 |
est31 |
its just not printed nicely :) |
07:12 |
est31 |
waited for SN's logging PR before touching that |
07:12 |
Hunterz |
ah ok |
07:13 |
est31 |
the logging PR has an extra method for it |
07:13 |
hmmmm |
SN's logging pr is sort of poorly structured |
07:14 |
hmmmm |
do you notice how there's logToSystem and logToOutputs |
07:14 |
hmmmm |
...is the system not an output?! |
07:15 |
hmmmm |
so there should be a SystemLogBuffer in addition to the LogBuffer/RawLogBuffer and then I guess the ncurses console output gets a new kind of Buffer |
07:15 |
hmmmm |
NCursesBuffer ?? |
07:16 |
est31 |
ewww |
07:16 |
est31 |
he removed functionality I have used before :( |
07:16 |
hmmmm |
yeah I don't know man |
07:16 |
hmmmm |
SN's "refactorings" don't always seem to be 100% helpful |
07:16 |
hmmmm |
I have mixed feelings on merging any more of them |
07:17 |
|
cib0 joined #minetest-dev |
07:19 |
est31 |
ncurses manages its own buffer |
07:32 |
|
Calinou joined #minetest-dev |
07:33 |
est31 |
(at least my pr does) |
07:33 |
est31 |
ermm branch |
07:51 |
|
nrzkt joined #minetest-dev |
07:53 |
|
Krock joined #minetest-dev |
08:05 |
|
proller joined #minetest-dev |
08:09 |
est31 |
okay |
08:10 |
est31 |
I do a PR with the changes I think are neccessary for the ncurses PR |
08:23 |
|
DFeniks joined #minetest-dev |
08:41 |
|
proller joined #minetest-dev |
09:04 |
|
eeew joined #minetest-dev |
09:21 |
|
Megaf joined #minetest-dev |
09:37 |
|
ElectronLibre joined #minetest-dev |
10:00 |
|
rubenwardy joined #minetest-dev |
10:10 |
|
BlockMen joined #minetest-dev |
10:23 |
|
est31 joined #minetest-dev |
10:29 |
BlockMen |
can i push #3283 ? |
10:29 |
ShadowBot |
https://github.com/minetest/minetest/issues/3283 -- Fix on_rightclick() being called directly after placing node by BlockMen |
10:30 |
est31 |
have you tested it? |
10:30 |
est31 |
if yes then yes |
10:30 |
VanessaE |
return BlockMen.has_tested |
10:30 |
VanessaE |
:P |
10:31 |
BlockMen |
"true" |
10:31 |
est31 |
answer("can i push #3283 ?", "yes") |
10:31 |
ShadowBot |
https://github.com/minetest/minetest/issues/3283 -- Fix on_rightclick() being called directly after placing node by BlockMen |
10:33 |
VanessaE |
I can reproduce that bug also, fwiw. |
10:33 |
|
crazyR joined #minetest-dev |
10:34 |
BlockMen |
ok, done |
10:35 |
BlockMen |
any comments on #3209 and #3040? |
10:35 |
ShadowBot |
https://github.com/minetest/minetest/issues/3209 -- Disable backface culling for models; fixes #2984 by BlockMen |
10:35 |
ShadowBot |
https://github.com/minetest/minetest/issues/3040 -- Fix jittering sounds on entities (fixes #2974) by BlockMen |
10:35 |
BlockMen |
is there a specific reason its not merged yet? |
10:36 |
est31 |
no |
10:36 |
est31 |
the backface culling has the issues pilzadam pointed out I guess |
10:37 |
est31 |
most times you are not inside other players |
10:37 |
est31 |
but you want fps to be high :9 |
10:37 |
est31 |
s/9/\)/ |
10:37 |
BlockMen |
by theory there is an impact, i was not able to notice any. |
10:38 |
BlockMen |
second it influences all types of objects, mobs aswell |
10:38 |
est31 |
and dropped items |
10:39 |
BlockMen |
third we want be compatible to mc skins, so this is needed for |
10:39 |
BlockMen |
yes |
10:39 |
est31 |
there its only needed for players |
10:39 |
BlockMen |
mobs aswell, e.g. ghosts from creatures look much better |
10:40 |
est31 |
what about making it a flag as you proposed? |
10:40 |
est31 |
but default culling on? |
10:40 |
est31 |
(and culling off for players) |
10:40 |
BlockMen |
fine for me |
10:43 |
est31 |
and I agree with pilzadam its better to have whitespace fixes in a different commit |
10:47 |
BlockMen |
then we should add this to rules, as stated there aswell ;) |
10:48 |
est31 |
seems reasonable |
10:53 |
BlockMen |
ok, will add it to dev wiki then and wont mix it next time |
10:54 |
BlockMen |
can i merge #3040 otherwise? |
10:54 |
ShadowBot |
https://github.com/minetest/minetest/issues/3040 -- Fix jittering sounds on entities (fixes #2974) by BlockMen |
10:57 |
|
crazyR joined #minetest-dev |
11:00 |
est31 |
what have you changed |
11:00 |
BlockMen |
basically this: https://github.com/minetest/minetest/pull/3040/files#diff-efb51cde4bb3c805a95dc31b2b1781d0R331 |
11:01 |
BlockMen |
it checks in collisoncheck whether you stand on object |
11:01 |
BlockMen |
and stops playing the node sounds when standing on object instead of ground |
11:01 |
est31 |
well, it might not be really proper, but at least fixes that bug |
11:01 |
est31 |
later it can be found out how to fix the root case |
11:02 |
est31 |
but better to split the code cleanup out |
11:02 |
BlockMen |
really? |
11:02 |
est31 |
it makes the commit much easier to read |
11:04 |
BlockMen |
im hoestly not willing to fiddle around with it again |
11:05 |
BlockMen |
not sure where to add this "dont mix" statement. here? http://dev.minetest.net/Git |
11:05 |
|
Krock joined #minetest-dev |
11:05 |
BlockMen |
or here http://dev.minetest.net/Git_Guidelines#Upstream_pull_requests_and_issues |
11:05 |
est31 |
the first one is more of a help site |
11:06 |
est31 |
the second more a rules page |
11:06 |
est31 |
id take the second one |
11:08 |
BlockMen |
added |
11:12 |
|
CraigyDavi joined #minetest-dev |
11:14 |
|
Darcidride joined #minetest-dev |
11:25 |
|
Lunatrius` joined #minetest-dev |
11:38 |
BlockMen |
bbl |
11:51 |
|
Amaz joined #minetest-dev |
11:58 |
|
crazyR joined #minetest-dev |
12:15 |
|
DFeniks joined #minetest-dev |
12:25 |
harrison |
can we talk about raytracing and voxels and stuff? please? |
12:25 |
harrison |
after all it is the future of realtime graphics -- it just has to be! |
13:07 |
|
Warr1024 joined #minetest-dev |
13:38 |
|
younishd joined #minetest-dev |
13:51 |
|
PilzAdam joined #minetest-dev |
14:56 |
|
est31 joined #minetest-dev |
15:07 |
|
zat joined #minetest-dev |
15:22 |
|
VanessaE joined #minetest-dev |
15:26 |
|
Player2 joined #minetest-dev |
15:51 |
|
zat joined #minetest-dev |
16:01 |
|
crazyR joined #minetest-dev |
16:03 |
|
est31 joined #minetest-dev |
16:09 |
est31 |
awww |
16:09 |
est31 |
a subtle bug! |
16:10 |
|
crazyR joined #minetest-dev |
16:12 |
VanessaE |
a bug? say it ain't so! |
16:12 |
VanessaE |
:P |
16:13 |
Calinou |
it's notabug.org, it's a feature |
16:13 |
Calinou |
:) |
16:13 |
est31 |
well until now rebasing my terminal was pretty easy |
16:13 |
est31 |
basically rebase |
16:13 |
est31 |
but thanks to threading AND logging changes... |
16:13 |
est31 |
well I've asked for them |
16:13 |
est31 |
die geister die ich rief... |
16:14 |
est31 |
(to be merged) |
16:18 |
Hunterz |
terminal console going to upstream ? |
16:19 |
|
hmmmm joined #minetest-dev |
16:23 |
|
crazyR joined #minetest-dev |
16:23 |
|
est31 joined #minetest-dev |
16:37 |
|
Gael-de-Sailly joined #minetest-dev |
16:44 |
|
crazyR joined #minetest-dev |
16:59 |
|
paramat joined #minetest-dev |
17:03 |
paramat |
PilzAdam perhaps #3271 should be merged soon? |
17:03 |
ShadowBot |
https://github.com/minetest/minetest/issues/3271 -- Lua settings improvements by PilzAdam |
17:04 |
PilzAdam |
yes, I'm waiting for someone to tell me to do it |
17:04 |
nrzkt |
PilzAdam, please squash |
17:04 |
|
BlockMen joined #minetest-dev |
17:06 |
VanessaE |
that PR is gonna be a bitch to squash properly..... |
17:08 |
Krock |
hmmmm, what happened to 3281? Can I strikethrough that compiling request from my todo list? |
17:09 |
hmmmm |
yeah, the threading compilation fix was on my list of things TODO with 3281 |
17:09 |
hmmmm |
I suppose you could rebase your PR to fix the gettext compilation too, but it's probably not worth it because I'm going to refactor that piece anyway |
17:10 |
hmmmm |
like, that really should not be a platform-conditional function definition |
17:10 |
hmmmm |
it hides problems exactly like what you just fixed |
17:10 |
Krock |
Oh well, if you're going to change code there it's nonsense to rebase my pull request |
17:10 |
hmmmm |
sorry :( |
17:13 |
PilzAdam |
nrzkt, done |
17:13 |
PilzAdam |
VanessaE, why? it worked fine for me |
17:13 |
hmmmm |
yeah, so what is 3271 waiting on exactly? |
17:13 |
VanessaE |
PilzAdam: well I wasn't counting on you squashing it into just one commit :) |
17:13 |
hmmmm |
I'm not 100% sure I understand what that PR does so I'm withholding judgement |
17:14 |
PilzAdam |
hmmmm, it does good things ;-) |
17:14 |
VanessaE |
(to me, "proper" in this context would be... well basically split and merge them up according to the 9 items in your "Changes") |
17:14 |
hmmmm |
Yeah, you know what, I really don't think this is something that should be squashed |
17:14 |
VanessaE |
too late :P |
17:14 |
hmmmm |
PilzAdam, are you able to revert back to the old history or is it too late? |
17:15 |
PilzAdam |
I'm not going to sqash this into several commits |
17:15 |
PilzAdam |
if someone is willing to invest some hours on this then feel free to |
17:15 |
hmmmm |
I didn't see the actual commit log for this branch |
17:15 |
hmmmm |
was it like... |
17:15 |
VanessaE |
translation: "well it would indeed be a bitch to do if I did it like that" ;) |
17:15 |
hmmmm |
"Improve lua settings" |
17:16 |
hmmmm |
"Fix this tab on this line" |
17:16 |
hmmmm |
"Rename this variable" |
17:16 |
hmmmm |
etc.. |
17:16 |
hmmmm |
or was it |
17:16 |
hmmmm |
"Do one actual thing here" |
17:16 |
hmmmm |
"Implement other piece of functionality here" |
17:16 |
PilzAdam |
hmmmm, a bit of both |
17:16 |
hmmmm |
the former of the two situations is where a PR's commit log should be squashed, the second not so much |
17:17 |
hmmmm |
I really wish you people would start making more regular commits and not jamming it all into one huge opaque package |
17:17 |
PilzAdam |
I would prefer that too, but other devs seem to prefer this |
17:17 |
hmmmm |
listen to what makes more sense |
17:18 |
Krock |
hmmmm, rebased the pull - as a fallback fix |
17:19 |
hmmmm |
if people keep doing this, we're inevitably going to merge a big "refactor" PR that'll essentially be the minetest equivalent of the TTIP |
17:19 |
PilzAdam |
so, paramat, what about merging #3271 now? |
17:19 |
ShadowBot |
https://github.com/minetest/minetest/issues/3271 -- Lua settings improvements by PilzAdam |
17:19 |
|
est31 joined #minetest-dev |
17:20 |
PilzAdam |
hmmmm, btw, what 3271 does: 1) it expands descriptions on a bunch of settings 2) it adds new types and better input mechanisms 3) it gives mods / games the ability to define their own settingtypes.txt |
17:20 |
PilzAdam |
that would sum it pretty much up |
17:21 |
est31 |
my position on this is that at the time of merging, the history of a pr should be clearly separated in topics, and large prs (like the logging or threading pr by SN) should be split, but the history shouldn't reflect how the PR was created, but rather reflect the topics. |
17:21 |
hmmmm |
well |
17:21 |
est31 |
and I rather like a single big commit msg than 40 small "update to est's suggestions" "fix crash by previous commit" etc |
17:21 |
hmmmm |
I guess this can be okay, I don't particularily like 2000 line change commits |
17:22 |
hmmmm |
no doubt this PR has been brewing for quite some time |
17:22 |
est31 |
I know however that it is hard to make a faked history so I tell people to squash because they'll do that rather than create a clean sane history |
17:22 |
hmmmm |
but let it be a warning for next time - this gigantic, impossible-to-review, opaque PR stuff needs to stop |
17:23 |
PilzAdam |
I personally would prefer a system where people use actual dev branches with proper commits, and we merge them with merge commits |
17:23 |
hmmmm |
that has its own big problems |
17:23 |
est31 |
I don't like that. |
17:24 |
hmmmm |
I would argue the problems with that sort of workflow are worse than the problems of our current setup |
17:24 |
est31 |
you have tons of "merge this merge that" |
17:24 |
est31 |
a faked history is something more "higher" of a good as it allows you to see what a commit actually does |
17:24 |
hmmmm |
and not only that, but commits show in the history in order of their timestamp, not when they were actually added |
17:24 |
est31 |
and not just an entry in a changelog |
17:26 |
est31 |
the project is to small both in code base as well as team size for such a change to be neccessary imo |
17:27 |
hmmmm |
even with large teams/codebases I'd prefer this setup... |
17:27 |
hmmmm |
otherwise it's just a total mess |
17:27 |
est31 |
tbh I dont really have experiences with large codebases, so my opinion isnt very funded there |
17:29 |
hmmmm |
init_gettext is a freaking wreck |
17:29 |
hmmmm |
who wrote this initially |
17:30 |
paramat |
PilzAdam the parts of 3271 i understand look good +1 |
17:30 |
hmmmm |
I recommend somebody refactor this to not have so many damn conditional compilation statements - this is nuts |
17:32 |
hmmmm |
https://github.com/kwolekr/minetest/commit/5b262785b2312e49c2faf222b0389b89ea1c79b0 PTAL |
17:32 |
est31 |
I've looked at large parts of 3271 it seems good now |
17:33 |
hmmmm |
note to everybody - stop doing the kind of crap I just removed in this commit - it is not helpful |
17:34 |
est31 |
looks good |
17:35 |
est31 |
PTAL #3290 |
17:35 |
ShadowBot |
https://github.com/minetest/minetest/issues/3290 -- Small logging refactor and additional options by est31 |
17:36 |
hmmmm |
paramat: I gave you the thumbs up for 3284 btw |
17:36 |
hmmmm |
did you see that? |
17:36 |
paramat |
yeah thanks |
17:38 |
|
Amaz joined #minetest-dev |
17:38 |
PilzAdam |
paramat, est31, so merge now? |
17:38 |
PilzAdam |
it's already rebased on master |
17:38 |
paramat |
sure |
17:38 |
est31 |
ok |
17:40 |
PilzAdam |
done |
17:41 |
paramat |
PilzAdam sfan5 game#713 for whenever you have time to consider |
17:41 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/713 -- Default/functions: ABM for moss growth on cobble near water by paramat |
17:41 |
VanessaE |
paramat: ^^ you never addressed my comments about that |
17:42 |
PilzAdam |
paramat, I like what it does, I like the code, but I haven't tested it |
17:42 |
PilzAdam |
so if it's tested, then +1 |
17:42 |
paramat |
okay, yes it's tested |
17:43 |
paramat |
VanessaE you would like checks for air and water? |
17:43 |
PilzAdam |
sfan5, game#709 can now be merged |
17:43 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/709 -- Add settingtypes.txt by PilzAdam |
17:44 |
VanessaE |
paramat: not necessarily - rather you should add something that will allow me to rewrite gloopblocks to totally eliminate its mossy feature entirely, i.e. register_mossy("stone", "gloopblocks:stone_mossy") or something similar |
17:45 |
VanessaE |
(though having the target node being bound by both air and water would be a good idea. I've seen what happens when it isn't :) ) |
17:54 |
est31 |
okay hmmmm updated my pr |
17:54 |
hmmmm |
LGTM |
17:56 |
PilzAdam |
Krock, can you rebase #3267 ? |
17:56 |
ShadowBot |
https://github.com/minetest/minetest/issues/3267 -- Standardize the menu button order and sizes by SmallJoker |
17:56 |
Krock |
<Krock>rebase time! |
17:57 |
Krock |
trying to find optimal positions |
17:57 |
Hunterz |
with #3290 ncurses console get timestamp right? |
17:57 |
ShadowBot |
https://github.com/minetest/minetest/issues/3290 -- Small logging refactor and additional options by est31 |
17:58 |
est31 |
yes, its planned |
17:58 |
Hunterz |
nice |
17:59 |
paramat |
hm the 2nd abm in that commit could cause some 'buried' mossycobble, but the fix needs 1 or 2 extra scans, it might not be worth it. we could perhaps consider moss has penetrated into a thick cobble wall through cracks |
17:59 |
paramat |
moss underwater seems okay |
18:00 |
VanessaE |
moss deep underwater isn't |
18:00 |
VanessaE |
in practice, it doesn't look good |
18:02 |
Krock |
PilzAdam, done. Check it out., |
18:04 |
PilzAdam |
est31, you kinda handle translations currently, right? |
18:04 |
est31 |
yup |
18:04 |
PilzAdam |
can you run updatepo.sh soon-ish? |
18:05 |
PilzAdam |
all the new setting comments need to be translated |
18:05 |
est31 |
don't we want to fix typos in settingstypes.txt etc first? |
18:05 |
Krock |
^ |
18:05 |
est31 |
otherwise translations will get the string with the typo |
18:05 |
est31 |
then its translated |
18:05 |
est31 |
then the typo gets fixed |
18:06 |
est31 |
then the translator has to look at it again |
18:06 |
est31 |
at least to remove the fuzzy flag |
18:07 |
PilzAdam |
true, but AFAIK there no typos left |
18:07 |
PilzAdam |
but I guess / hope many people will make PRs to improve comments |
18:08 |
PilzAdam |
Krock, I would expect the buttons to be aligned right, not in the center |
18:08 |
PilzAdam |
est31, I guess translating the 80% of the comments that will stay the same is worth it, though |
18:08 |
Krock |
arabic languages might disagree |
18:09 |
PilzAdam |
do you want to run the positions through gettext? |
18:09 |
est31 |
okay found another reason to stall it lol: can you have a look at #3230 so that it can be merged before updatepo runs ? |
18:09 |
ShadowBot |
https://github.com/minetest/minetest/issues/3230 -- Better gettext support for protocol version mismatch messages by est31 |
18:10 |
PilzAdam |
est31, that code is absolutely unreadable |
18:11 |
paramat |
central buttons look better to me |
18:12 |
PilzAdam |
est31, can you split that up into seperate ifs? |
18:13 |
PilzAdam |
so don't use "string" .. condition and "string" |
18:13 |
PilzAdam |
rather use: var = "string"; if condition then var = var .. "string" |
18:14 |
BlockMen |
#3267 why does it have to be the same location everywhere? In simple dialogs centered looks good but in the config dialog it looks displaced IMO |
18:14 |
ShadowBot |
https://github.com/minetest/minetest/issues/3267 -- Standardize the menu button order and sizes by SmallJoker |
18:16 |
PilzAdam |
BlockMen, true |
18:16 |
Krock |
:< |
18:17 |
est31 |
PilzAdam, updated |
18:17 |
BlockMen |
est31, #3209 good now? |
18:17 |
ShadowBot |
https://github.com/minetest/minetest/issues/3209 -- Add option to disable backface culling for models by BlockMen |
18:17 |
PilzAdam |
Krock, the "start" buttons in singleplayer, client and server tab are right aligned |
18:17 |
PilzAdam |
you should be consistent with that |
18:18 |
PilzAdam |
est31, it's good now +1 |
18:18 |
Krock |
PilzAdam, that's logical because it's related to the selection box above |
18:19 |
PilzAdam |
est31, you can merge that now |
18:19 |
est31 |
okay |
18:19 |
est31 |
thx |
18:20 |
|
crazyR joined #minetest-dev |
18:21 |
est31 |
okay found at least one typo in settingstypes.txt "Smooths" |
18:22 |
est31 |
right version is "Smoothes" |
18:22 |
est31 |
at least according to what I think |
18:22 |
* est31 |
is native no speaker |
18:22 |
Krock |
^ is native writer |
18:24 |
PilzAdam |
est31, rubenwardy had some remarks on some things, too |
18:24 |
est31 |
should I run the updatepo, or should we wait till rubenwardy's requests are met? |
18:25 |
PilzAdam |
I would run it now |
18:25 |
PilzAdam |
(I want to test the weblate interface ;-)) |
18:26 |
est31 |
okidoki |
18:28 |
est31 |
note that it takes the email address you use for sign up for the commits |
18:28 |
est31 |
so if thats private, change it :) |
18:29 |
PilzAdam |
I signed in via github |
18:30 |
est31 |
then its only an issue if you have turned on githubs email hiding feature |
18:30 |
est31 |
which it ignores as well :) |
18:31 |
est31 |
so there it is, pushed |
18:32 |
PilzAdam |
will weblate update automatically? |
18:32 |
est31 |
yes |
18:32 |
est31 |
idk whether there is a button for it |
18:32 |
est31 |
ah "Pull" |
18:32 |
est31 |
yup its in there |
18:33 |
est31 |
loool |
18:33 |
est31 |
the score just decreased for all strings |
18:34 |
est31 |
for german, from ~100% to ~30% |
18:34 |
PilzAdam |
that's why I asked to update the languages so early |
18:35 |
est31 |
hrmm multiline msgids |
18:36 |
est31 |
will have to make my vandalism checker tool ready for that |
18:36 |
* est31 |
has to do so much things |
18:36 |
est31 |
in so little time... :/ |
18:36 |
|
crazyR joined #minetest-dev |
18:37 |
|
paramat left #minetest-dev |
18:45 |
est31 |
PTAL https://github.com/est31/minetest/commit/719f0fc56af81e171643e219dae54cbada623c6d |
18:45 |
est31 |
I remove lotsa trailing whitespaces |
18:45 |
est31 |
and fix that bug pointed out by @arpruss |
18:45 |
est31 |
arpruss |
18:47 |
est31 |
if you want, I can do the whitespace removing in a separate commit |
18:48 |
est31 |
otherwise you can pass an argument to git diff to ignore whitespaces |
18:48 |
est31 |
I think it was --whitespace=ignore |
18:48 |
est31 |
okay better msg: https://github.com/est31/minetest/commit/49bda7f98d2ca1d3553460d27c7308856969fb44 |
18:49 |
est31 |
if nobody objects I'll push it in 10 minutes |
18:50 |
BlockMen |
est31, +1 |
19:31 |
PilzAdam |
est31, these are a lot of strings; I already work an hour on this and I'm only ~ 25% done |
19:32 |
est31 |
wow |
19:36 |
PilzAdam |
I have no idea how weblate sorts the strings |
19:37 |
est31 |
me neither |
19:42 |
PilzAdam |
I'm done for now |
19:42 |
est31 |
weblate will automatically commit your changes |
19:42 |
est31 |
and once a month or if there is another specific reason I look at weblate's repo and pull all the commits over |
19:42 |
est31 |
including hand-reviewing them |
19:43 |
est31 |
review mostly done bc vandalism |
19:43 |
rubenwardy |
why is french translation locked? |
19:43 |
est31 |
bc sb else translates right now |
19:43 |
est31 |
by kilbith it seems |
19:44 |
est31 |
(you have to hover over the lock icon) |
19:44 |
rubenwardy |
I didn't know who Jean Patrick GI was |
19:44 |
rubenwardy |
oh |
19:44 |
rubenwardy |
I didn't know who Jean Patrick G was |
19:48 |
PilzAdam |
I posted the translation stuff on the forums |
19:57 |
PilzAdam |
rubenwardy, https://github.com/minetest/minetest_game/pull/709#discussion_r42937687 I don't get it |
19:59 |
rubenwardy |
If you're trying to configure a server using the settings, it'd say "true" when it actually becomes "false". |
19:59 |
PilzAdam |
you would only see "true" if it's not set |
19:59 |
rubenwardy |
https://github.com/minetest/minetest_game/blob/master/mods/tnt/init.lua#L5 |
19:59 |
PilzAdam |
every other case is correct |
19:59 |
|
Icedream joined #minetest-dev |
20:00 |
nrzkt |
rubenwardy, it's kilbith yes |
20:00 |
PilzAdam |
this is because I can't set a different default value based on arbitrary conditions |
20:00 |
rubenwardy |
if it's not set, then you would see "true", but when you create a server it's "false. |
20:00 |
hmmmm |
[02:47 PM] <est31> if you want, I can do the whitespace removing in a separate commit |
20:00 |
hmmmm |
[02:48 PM] <est31> otherwise you can pass an argument to git diff to ignore whitespaces |
20:00 |
hmmmm |
don't do this |
20:00 |
rubenwardy |
It's a small issue though, not worth fixing |
20:00 |
hmmmm |
we made a decision a long time ago, it's so much easier to just leave the whitespace fixes in with your commit |
20:00 |
hmmmm |
just make sure you don't introduce any more, and the current whitespace errors are in the gradual process of being fixed |
20:01 |
PilzAdam |
est31, what happens when one creates a new language in the weblate interface? is everything correctly set up? |
20:01 |
hmmmm |
at some point I'm going to modify cpplint.py to suit our own style, which catches a lot of things |
20:01 |
|
Gael-de-Sailly joined #minetest-dev |
20:01 |
hmmmm |
and we'll make it a requirement to run this on your commit before merging anything |
20:01 |
hmmmm |
hopefully this will catch a lot of the low-level dumb errors that we shouldn't be wasting human time on |
20:02 |
est31 |
PilzAdam, I'm not sure, it somehow worked for some languages |
20:02 |
est31 |
eg lobjan was added that way |
20:02 |
PilzAdam |
est31, btw, remember to add new languages to the "language" enum in settingtypes.txt from now on |
20:03 |
est31 |
hmmmm, you might want to have a look at #3142 then |
20:03 |
ShadowBot |
https://github.com/minetest/minetest/issues/3142 -- Let travis check for git format mistakes by est31 |
20:04 |
hmmmm |
I suppose that's a good start, but it doesn't have very sharp teeth |
20:04 |
est31 |
PilzAdam, can't that be automated? |
20:04 |
est31 |
e.g. by having a lua callback get_language_list |
20:04 |
PilzAdam |
that would require hacks when parsing settingtypes.txt |
20:04 |
est31 |
and extra type for it |
20:04 |
PilzAdam |
the current design is that everything is statically defined in the file |
20:05 |
|
paramat joined #minetest-dev |
20:05 |
PilzAdam |
I already thought about adding some "hack_" types to solve different issues, but I don't really like it |
20:05 |
PilzAdam |
remember that this format is now part of our mod API |
20:06 |
est31 |
ah yeah |
20:07 |
est31 |
what about types that only we can use in the engine's settingtypes.txt? |
20:07 |
PilzAdam |
I mean, it can be done and I won't object it; but I just have a not so good feeling about it |
20:07 |
est31 |
I just know from experience that people won't update |
20:07 |
rubenwardy |
how about a "mods" folder, where enable security is a child, as a place for mods to put their settings? |
20:07 |
est31 |
I'll try to do it, but you know, on the long term... |
20:08 |
est31 |
there is one |
20:08 |
rubenwardy |
consider forcing mods to put all their settings in Server > Mods > Modname |
20:08 |
paramat |
i'll merge #3284 soon, after checks |
20:08 |
ShadowBot |
https://github.com/minetest/minetest/issues/3284 -- Mgfractal: Independent iterations and scale parameters by paramat |
20:10 |
est31 |
hmmmm, so, can it be merged? With all the shit commits removed ofc |
20:11 |
PilzAdam |
rubenwardy, currently the settingtypes.txt of mods and games is just appended to the list in the builtin settingtypes.txt |
20:11 |
PilzAdam |
putting them in categories of the builtin stuff would require a certain format of it |
20:11 |
rubenwardy |
I understand that |
20:11 |
rubenwardy |
you can't add settings to an existing group? |
20:11 |
rubenwardy |
only append? |
20:11 |
est31 |
btw there is a problem |
20:12 |
rubenwardy |
I guess that's fine :/ |
20:12 |
PilzAdam |
rubenwardy, the setting is a list, internally, not a tree |
20:12 |
rubenwardy |
I see |
20:12 |
PilzAdam |
that is easier when doing the formspec transformation |
20:12 |
est31 |
a mod could add an option "Improve speed 10x" which actually points to enable mod security |
20:12 |
est31 |
or disable |
20:12 |
est31 |
whatever |
20:12 |
est31 |
thats not good |
20:13 |
PilzAdam |
est31, it can't add itself to the trusted_mods, though |
20:13 |
rubenwardy |
but could turn it off? |
20:13 |
PilzAdam |
we could add checks to forbid double-adding of settings |
20:13 |
rubenwardy |
or adding of secure.* |
20:13 |
est31 |
yea both seem reasonable |
20:14 |
* PilzAdam |
starts coding |
20:14 |
rubenwardy |
double adding may be useful if a setting is relevant in two sections. However, that isn't good design |
20:15 |
est31 |
checking secure.* is problematic if its ever changed in the engine |
20:15 |
rubenwardy |
why would it be changed? |
20:15 |
est31 |
and forgotten to be changed in lua |
20:15 |
est31 |
dunno, perhaps sb wants to shorten it to sec. ? |
20:15 |
est31 |
or security. ? |
20:16 |
est31 |
or some other reason |
20:16 |
rubenwardy |
It's not just useful for security though |
20:16 |
PilzAdam |
I would expect people to grep to all source files in this case |
20:16 |
PilzAdam |
s/to/through/ |
20:16 |
rubenwardy |
also useful for URL whitelists, etc |
20:16 |
rubenwardy |
exactly |
20:16 |
est31 |
my command is usually grep -rIn "text" src |
20:17 |
est31 |
I never scan . bc it includes the build directory |
20:17 |
est31 |
and build includes lotsa stuff |
20:17 |
paramat |
now merging 3284 |
20:17 |
est31 |
(not in the repo, but all the android libraries that get downloaded during android build) |
20:17 |
est31 |
and ofc it doesnt get the symlink |
20:19 |
est31 |
there is a symlink in build/android/jni/src/ pointing to the src folder |
20:19 |
est31 |
so grepping in . will lead to the searched text appear twice |
20:19 |
est31 |
and will be super slow bc it also scans irrlicht and friends |
20:20 |
est31 |
yeah and mainmenu is in builtin/ |
20:21 |
paramat |
merge complete |
20:22 |
est31 |
5090 commits |
20:22 |
est31 |
thats 10 from 5100 |
20:22 |
est31 |
it was like yesterday when we had commit 5k |
20:32 |
PilzAdam |
https://github.com/PilzAdam/minetest/commit/9ee0d376d4e4679be074ed3a27e9700169aecea3 |
20:33 |
est31 |
why is the diff so large? |
20:33 |
PilzAdam |
:-/ this diff is kinda huge due to some simple restructuring |
20:34 |
PilzAdam |
basically return directly when encountering an error, instead of continuing in nested ifs |
20:39 |
paramat |
i have a request for hmmmm or anyone #2977 this is beyond my capability. much needed for floatland realms, alternative mapgens and travel by water column |
20:39 |
ShadowBot |
https://github.com/minetest/minetest/issues/2977 -- Stop liquids from flowing over ignore. by red-001 |
20:42 |
PilzAdam |
est31, can I merge that soon or should I create a PR? |
20:44 |
est31 |
looks good |
20:44 |
est31 |
so good for merge |
20:44 |
est31 |
if tested |
20:45 |
PilzAdam |
ok, I'll wait a few minutes to give others a chance to say something, too |
20:46 |
nrzkt |
#3287 if you want est31 |
20:46 |
ShadowBot |
https://github.com/minetest/minetest/issues/3287 -- Credits: Remove my name by Rui914 |
20:46 |
est31 |
yea seen it, I'm busy right now with rebasing my console |
20:47 |
est31 |
getting the weird error that verbosestream works |
20:47 |
est31 |
but actionstream doesn#t |
20:47 |
est31 |
s/#/'/ |
20:51 |
PilzAdam |
est31, does it fail to compile or isn't it logging stuff? |
20:51 |
est31 |
I made compiling work |
20:51 |
est31 |
so that works again |
20:52 |
est31 |
verbosestream works really fine |
20:52 |
est31 |
added a log emit to each server step |
20:52 |
est31 |
now trying to set the log level of actionstream to LL_VERBOSE |
20:52 |
est31 |
yeey it works |
20:53 |
est31 |
now replacing it with action again, and applying the hack at another position |
20:54 |
PilzAdam |
pushing https://github.com/PilzAdam/minetest/commit/9ee0d376d4e4679be074ed3a27e9700169aecea3 now |
21:04 |
est31 |
no |
21:04 |
est31 |
dont say this is true |
21:05 |
* est31 |
rebuilding to check with valgrind |
21:05 |
|
Icedream joined #minetest-dev |
21:11 |
PilzAdam |
oops |
21:11 |
est31 |
how can you make valgrind check for uninitialized reads? |
21:12 |
PilzAdam |
est31, https://github.com/PilzAdam/minetest/commit/7d5c7365314c9517369a7a7d6e4fc5d8712b93c7 |
21:12 |
PilzAdam |
memcheck does that default, AFAIK |
21:12 |
PilzAdam |
+by |
21:12 |
PilzAdam |
this commit fixes a bug in my last commit |
21:13 |
est31 |
ok +1 |
21:13 |
est31 |
whats the difference |
21:14 |
PilzAdam |
settings.current_comment is always nil in this place |
21:14 |
PilzAdam |
while the local variable current_comments contains the actual comment |
21:15 |
est31 |
hrmm isnt it "" ? |
21:15 |
est31 |
https://github.com/PilzAdam/minetest/commit/7d5c7365314c9517369a7a7d6e4fc5d8712b93c7#diff-b61e3e14dbe70423022f4ee065fa3f94L44 |
21:15 |
PilzAdam |
eh, yes |
21:15 |
PilzAdam |
whatever |
21:15 |
PilzAdam |
it's late |
21:15 |
est31 |
yea 4 me too |
21:16 |
est31 |
if that actionstream bug is fixed I update my branch and hope the best xD |
21:16 |
PilzAdam |
you shouldn't be allowing me to push upstream after 22:00, btw |
21:16 |
est31 |
meh, I do mistakes as well |
21:17 |
est31 |
my specialty is making gramatically correct commit msgs with words swapped |
21:17 |
est31 |
https://github.com/minetest/minetest/commit/49bda7f98d2ca1d3553460d27c7308856969fb44 |
21:17 |
est31 |
bad word is "statically" |
21:18 |
est31 |
should be "dynamically" |
21:19 |
PilzAdam |
that's basically the same |
21:19 |
est31 |
man how can this happen |
21:20 |
est31 |
I do know that m_silenced_levels is at the index of LL_ACTION set to true |
21:20 |
est31 |
now I wonder where it is set to true |
21:20 |
est31 |
but valgrind reports no uninitialized read |
21:20 |
est31 |
and setting it to false has no effect |
21:21 |
est31 |
so somehow it is initialized before first read |
21:21 |
est31 |
and after my failed attempt to change it to false :) |
21:21 |
est31 |
now the only place where that private member is set in class is setLevelSilenced |
21:22 |
est31 |
but that method is never called outside testing |
21:22 |
est31 |
okay next try |
21:22 |
est31 |
perhaps volatile doesnt reserve memory |
21:23 |
est31 |
and it shares memory with the map below |
21:23 |
* est31 |
removes volatile |
21:24 |
|
Hunterz joined #minetest-dev |
21:27 |
|
paramat left #minetest-dev |
21:51 |
|
crazyR joined #minetest-dev |
21:52 |
PilzAdam |
est31, there is a bug where " are not escaped in the settings translation file |
21:53 |
PilzAdam |
I will fix this tomorrow; the translation files need to be updated again, then |
22:04 |
est31 |
okay, anybody here? |
22:06 |
BlockMen |
wanted leave now |
22:06 |
BlockMen |
y? |
22:06 |
est31 |
can you look at https://github.com/est31/minetest/commit/fae1eb9a1969194c61feb25312fdbb5d397c0d8a |
22:07 |
|
zat joined #minetest-dev |
22:08 |
est31 |
took me hours to find the cause. |
22:11 |
BlockMen |
is the int conversion necessary? |
22:11 |
BlockMen |
otherwise seems right, +1 |
22:11 |
est31 |
I think so |
22:11 |
est31 |
oh |
22:12 |
est31 |
not neccessary after all |
22:12 |
est31 |
removing it |
22:14 |
BlockMen |
ok, good. im going bed now. feel free to merge from my side |
22:15 |
est31 |
fine |
22:15 |
est31 |
I'll do that as well I think |
22:22 |
|
domtron joined #minetest-dev |
23:07 |
|
est31 joined #minetest-dev |
23:07 |
est31 |
okay, there it is: #3292 |
23:07 |
ShadowBot |
https://github.com/minetest/minetest/issues/3292 -- Add server side ncurses terminal by est31 |
23:07 |
est31 |
still missing a polish, but ok for a first glimpse and an overview of general architecture |
23:52 |
hmmmm |
lol |
23:52 |
hmmmm |
est31: nice catch |
23:52 |
hmmmm |
I missed this too when reviewing it |
23:55 |
hmmmm |
[04:10 PM] <est31> hmmmm, so, can it be merged? With all the shit commits removed ofc -- huh? |
23:56 |
hmmmm |
oh you mean squashed? |
23:56 |
hmmmm |
yeah, see that's an example of a PR that needs squashing |
23:57 |
hmmmm |
lol, https://github.com/est31/minetest/commit/91e70b0a37c6dac86defdad7db56c97906149ac1 |
23:57 |
hmmmm |
inspired by Gerogerigegege |