Time |
Nick |
Message |
00:03 |
|
halt_ joined #minetest-dev |
00:04 |
|
halt_ joined #minetest-dev |
00:06 |
|
halt_ joined #minetest-dev |
00:07 |
|
halt_ joined #minetest-dev |
00:09 |
|
halt_ joined #minetest-dev |
00:10 |
|
halt_ joined #minetest-dev |
00:41 |
|
Puka joined #minetest-dev |
00:44 |
|
Tmanyo joined #minetest-dev |
01:54 |
|
CalebDavis joined #minetest-dev |
01:57 |
|
Miner_48er joined #minetest-dev |
03:44 |
|
jsgrant_ joined #minetest-dev |
05:25 |
|
Raven262 joined #minetest-dev |
06:09 |
|
QwertyDragon joined #minetest-dev |
06:25 |
|
Hunterz joined #minetest-dev |
06:42 |
VanessaE |
http://www.minetest.net/customize/ <-- dreambuilder should be removed from this page (it is not a subgame). carbone, BFD, and voxelgarden are all dead |
06:42 |
VanessaE |
(under "popular subgames"_ |
06:42 |
VanessaE |
) |
07:15 |
|
nerzhul joined #minetest-dev |
07:37 |
|
jin_xi joined #minetest-dev |
07:48 |
|
DS-minetest joined #minetest-dev |
07:57 |
|
Krock joined #minetest-dev |
08:12 |
|
kilbith joined #minetest-dev |
08:44 |
|
cx384 joined #minetest-dev |
09:01 |
|
Krock joined #minetest-dev |
09:21 |
Krock |
any plans of implementing this mapgen? http://imgur.com/gallery/0nFro :P |
09:26 |
Krock |
sadly, a mapblock couldn't be regenerated the same way if it was deleted |
09:44 |
red-001 |
could someone review #5941 or #5852? |
09:44 |
ShadowBot |
https://github.com/minetest/minetest/issues/5941 -- [CSM] allow the client to access and modify settings in the `client.` namespace. by red-001 |
09:44 |
ShadowBot |
https://github.com/minetest/minetest/issues/5852 -- Improve the path select GUI by red-001 |
10:14 |
|
YuGiOhJCJ joined #minetest-dev |
10:47 |
Krock |
red-001, there's an unanswered question on 5852 |
10:53 |
red-001 |
I'm not sure, I'm pretty sure someone requested that I update the files but I can't remeber who, I can revent that part of the commit if it's an issue |
11:04 |
|
Fixer joined #minetest-dev |
11:10 |
|
nerzhul joined #minetest-dev |
11:11 |
red-001 |
Krock, revented the change and rebased + squashed the PR |
11:11 |
|
kilbith joined #minetest-dev |
11:12 |
Krock |
s/revent/revert/ |
11:13 |
nerzhul |
merging #5946 in #10 mins |
11:13 |
ShadowBot |
https://github.com/minetest/minetest/issues/5946 -- Order es_DrawType exactly like enum NodeDrawType in nodedef.h by Thomas--S |
11:13 |
ShadowBot |
https://github.com/minetest/minetest/issues/10 -- Fixed null pointer dereference errors found by cppcheck by joshbeck |
11:13 |
nerzhul |
oops why i setted a # befor 10 :p |
11:13 |
nerzhul |
before* |
11:14 |
|
cx384 joined #minetest-dev |
11:16 |
nerzhul |
#5948 sounds good now Krock no ? |
11:16 |
ShadowBot |
https://github.com/minetest/minetest/issues/5948 -- Add a server-sided way to remove color codes from incoming chat messages by red-001 |
11:40 |
nerzhul |
merging #5948 #5604 & #5945 in ~10 mins |
11:40 |
ShadowBot |
https://github.com/minetest/minetest/issues/5948 -- Add a server-sided way to remove color codes from incoming chat messages by red-001 |
11:40 |
ShadowBot |
https://github.com/minetest/minetest/issues/5604 -- lua_api: Correct craft recipe param `type` to `method` by DS-Minetest |
11:40 |
ShadowBot |
https://github.com/minetest/minetest/issues/5945 -- C++11 patchset 6: forbid object copy using assigment/copy function delete by nerzhul |
11:53 |
|
DS-minetest joined #minetest-dev |
11:56 |
nerzhul |
if somes wants to comment #5953 |
11:56 |
ShadowBot |
https://github.com/minetest/minetest/issues/5953 -- Remove JThread license and add MT license header by nerzhul |
12:10 |
|
Taoki joined #minetest-dev |
13:49 |
|
kilbith_ joined #minetest-dev |
14:32 |
|
srifqi joined #minetest-dev |
14:55 |
|
Megaf joined #minetest-dev |
14:55 |
|
calcul0n joined #minetest-dev |
15:03 |
|
Player_2 joined #minetest-dev |
15:23 |
|
Megaf joined #minetest-dev |
15:39 |
|
KaadmY joined #minetest-dev |
17:15 |
|
nerzhul joined #minetest-dev |
17:27 |
ShadowNinja |
Krock, sfan5, rubenwardy, nore, celeron55, nerzhul: Meeting in about half an hour. |
17:44 |
|
paramat joined #minetest-dev |
18:02 |
ShadowNinja |
Alright, meeting starts now. |
18:02 |
VanessaE |
soi |
18:02 |
VanessaE |
so |
18:02 |
VanessaE |
first order of business |
18:02 |
VanessaE |
put the PB&J Pup back into minetest_game, without the animations and sound effects. |
18:02 |
ShadowNinja |
VanessaE: _game meeting. |
18:03 |
ShadowNinja |
Calinou: Any progress on the website pictures? |
18:04 |
ShadowNinja |
First PR is to fix the shader label alignment #5820 |
18:04 |
ShadowBot |
https://github.com/minetest/minetest/issues/5820 -- Menu: Do not use textlist for shaders in settings tab by octacian |
18:05 |
ShadowNinja |
Positioning might need a bit of tweaking, but the formspec scaling might make it impossible to get it lined up perfectly for everyone, so I wouldn't worry about it too much. |
18:05 |
Calinou |
ShadowNinja: no :( |
18:05 |
Krock |
but this PR already improves it a bit |
18:05 |
Krock |
better than nothing, at least |
18:05 |
ShadowNinja |
Is it possible to display a disabled checkbox? That would always line up, but I'm net sure if formspecs support it. |
18:06 |
Calinou |
don't shaders have that already? in the options |
18:06 |
ShadowNinja |
Calinou: Alright, well... whenever you get the time. |
18:06 |
Calinou |
ShadowNinja: yeah, I still have an internship going on right now, I should be on summer break on June 27th |
18:06 |
Krock |
"checkbox[<X>,<Y>;<name>;<label>;<selected>]" - no "enabled" arg available yet |
18:06 |
ShadowNinja |
Calinou: Nah, they turn into a text list. No checkboxes at all and they don't line up properly (See pics in PR). |
18:07 |
Calinou |
ah, I forgot |
18:07 |
* ShadowNinja |
mumbles something about how formspecs have to go. |
18:09 |
ShadowNinja |
Krock: Do you want to test and merge it? |
18:09 |
Krock |
already tested it. (that's where my image comes from) |
18:10 |
ShadowNinja |
Krock: Alright. It lines up pretty well? |
18:10 |
Krock |
yes |
18:11 |
Krock |
I think getting it done better will require checkboxes that can be disabled |
18:11 |
Krock |
will merge in a minute |
18:11 |
ShadowNinja |
Krock: Great, go ahead and merge then. Next is removeing superfluous null checks #5914. Supposedly requires a rebase, but GitHub disagrees. |
18:11 |
ShadowBot |
https://github.com/minetest/minetest/issues/5914 -- Removed redundant pointer null checks by QrchackOfficial |
18:11 |
Krock |
nerzhul, are you there? |
18:12 |
Krock |
5914 needs improvement - and maybe a rebase |
18:13 |
ShadowNinja |
Looks like 5914 is pretty simple. I can merge it. |
18:14 |
ShadowNinja |
Krock: What changes does it need? Just looks like there's a spacing change request -- and that's pretty pedantic. |
18:15 |
Krock |
there was something about the shared pointer being removed |
18:15 |
Krock |
but I'm fine with the PR if it's mergeable |
18:16 |
ShadowNinja |
Yeah, it was removed in the PR and trunk, but looks like the conflict has been resolved. |
18:16 |
Krock |
interesting |
18:16 |
ShadowNinja |
Another to consider is whether to remove minetestmapper.py #5901 |
18:16 |
ShadowBot |
https://github.com/minetest/minetest/issues/5901 -- Remove minetestmapper from this repository by nerzhul |
18:17 |
Krock |
Either move to the mapper repository or keep as-is. The code shouldn't be thrown away IMO |
18:18 |
ShadowNinja |
I don't think anyone uses it anymore, and it's still in the git history if anyone wants it. So I'm fine with just removing it. |
18:18 |
ShadowNinja |
Or it could get its own repo -- but it will probably never be updated. |
18:18 |
Krock |
okay, nerzhul and paramat share the same opinion. Well then, remove it. |
18:19 |
paramat |
since its preserved in history removal is ok with me |
18:20 |
Krock |
but the PR is incomplete, the colors file should be removed aswell |
18:20 |
ShadowNinja |
Yes. |
18:21 |
paramat |
and sectors2sqlite.py? |
18:21 |
kilbith |
I'm going to close #5457 |
18:21 |
ShadowBot |
https://github.com/minetest/minetest/issues/5457 -- GUI: Allow texture packs to customize the mouse pointer by kilbith |
18:22 |
ShadowNinja |
paramat: Yeah, that could probably be removed too. Doubt anyone's used sectors in a long time. |
18:22 |
ShadowNinja |
Next PR: Minetest apparently has a built-in file browser, but it can't select folders: #5852 |
18:22 |
ShadowBot |
https://github.com/minetest/minetest/issues/5852 -- Improve the path select GUI by red-001 |
18:24 |
rubenwardy |
Irrlicht has an inbuilt file browser :P |
18:24 |
Krock |
yes, that's what we use. But the directory selecting feature wasn't working right |
18:24 |
Krock |
or still isn't |
18:25 |
rubenwardy |
I suggest finding a wrapper library for native file browsers |
18:25 |
ShadowNinja |
rubenwardy: Yeah, but it resets the locale every time you open it. That's why the "local mod" option was removed from the mod store tab. |
18:25 |
rubenwardy |
when doing the Node Box Editor, I used tinyfiledialog - it's probably unmaintained by now |
18:27 |
ShadowNinja |
Qt works really well (native file pickers on most/all platforms), but it's a lot to bring in for just a file browser. |
18:27 |
ShadowNinja |
But fixing the GUI is a but too big of a topic for now I think. |
18:27 |
rubenwardy |
woah |
18:27 |
|
kaeza joined #minetest-dev |
18:27 |
rubenwardy |
tinyfiledialogs was last modified 13 hours ago |
18:27 |
rubenwardy |
https://sourceforge.net/projects/tinyfiledialogs/files/ |
18:30 |
ShadowNinja |
So does the path select PR look good? |
18:31 |
paramat |
no opinion |
18:34 |
ShadowNinja |
Looks OK to me. |
18:35 |
|
Puka joined #minetest-dev |
18:35 |
sfan5 |
does that pr introduce a new dependency? |
18:35 |
ShadowNinja |
sfan5: Nope, just some tweaks to existing code. |
18:36 |
sfan5 |
that's fine then |
18:37 |
sfan5 |
haven't looked at the code though yet |
18:37 |
ShadowNinja |
We also have #5819 to look at. |
18:37 |
ShadowBot |
https://github.com/minetest/minetest/issues/5819 -- Fix default item callbacks to work with nil users by raymoo |
18:40 |
nerzhul |
Krock, i'm eating with my family, late today, then i just read |
18:40 |
ShadowNinja |
The item_drop change looks pretty unrelated. |
18:40 |
nerzhul |
tell me what to remove on 5901 and i will update it |
18:40 |
Krock |
oh sorry for interrupting, nerzhul |
18:41 |
nerzhul |
no probme, as you see there is latency :p |
18:41 |
nerzhul |
problem* |
18:42 |
nerzhul |
ShadowNinja, for 5953 we removed all code for mutexes, and the threading code only has the class and some function declaration but definition has changed |
18:42 |
nerzhul |
look at 0.4.8 |
18:42 |
Krock |
nerzhul, you'll see the comments on github ;) colors.txt and sectors2sqlite.py can be removed |
18:43 |
nerzhul |
okay i will update this evening or tomorrow then |
18:43 |
Krock |
great. thanks :) |
18:45 |
ShadowNinja |
nerzhul: Yes, but I believe it's still legally under the JThread license if it was modified from JThread, even if there's lottle or nothing left of the original code. |
18:48 |
ShadowNinja |
Can we get a decision on timed move #1489 sfan5, rubenwardy, paramat? |
18:48 |
ShadowBot |
https://github.com/minetest/minetest/issues/1489 -- Timed move by sapier |
18:49 |
paramat |
i'm neutral |
18:50 |
Krock |
modders will always find an usage for this feature |
18:51 |
ShadowNinja |
I'd rather see it implemented in a more general way -- such as a movement queue or just CSM. |
18:51 |
sfan5 |
not everything needs to be csm |
18:51 |
sfan5 |
but a movement queue might be nice |
18:51 |
Krock |
indeed |
18:52 |
Krock |
"setvelocity({x=num, y=num, z=num}, time)" if "time" is set, add to the queue |
18:54 |
ShadowNinja |
I was thinking something more like q = obj.movement_queue(); q.add({type="move_to", pos={...}, time=1}); q.add({type="acceleration", acceleration={...}}) |
18:57 |
VanessaE |
ShadowNinja: we need to settle the issue about 0.4.x backports and point releases, now that the 0.5.x cycle has started |
18:57 |
Krock |
is that on the list? (where's the paste btw?) |
18:57 |
ShadowNinja |
VanessaE: Do you want to do a bugfix release? |
18:58 |
ShadowNinja |
Krock: No, but Iconsidered discussing it anyways. See -staff. |
18:58 |
VanessaE |
ShadowNinja: not necessarily. more a matter of "will we, won't we". i.e. let's make a decision about it now. |
18:58 |
paramat |
there will be a point release to sort out csm |
18:59 |
paramat |
other fixes could be in that |
18:59 |
VanessaE |
paramat: CSM is one issue that needs addressed, but I'm sure there will be other things as well |
18:59 |
Krock |
I believe nerzhul came up with this idea first. So we can release 0.4.17 and 0.5.0 parallel. the first just for bugfixes to play on older servers and the other is the actual thing |
18:59 |
VanessaE |
Krock: yes, but no one here has actually *agreed* on doing it |
18:59 |
|
DS-minetest joined #minetest-dev |
18:59 |
Krock |
that's why we're discussing it here now |
18:59 |
paramat |
Krock that's a separate proposal |
19:00 |
paramat |
red suggests a point release to restrict csm in around 2 months, maybe less |
19:01 |
ShadowNinja |
At some point we should go through the CSM API and make sure that everything in there is what we want, and make some sort of proper seperation between mods (I believe it does the same thing as the server ATM -- just running all mods in one lua state. I think it also uses the server security module -- which is more lenient than it probably should be since it was focused on backwards compat for the server. |
19:02 |
VanessaE |
paramat: I'd say 4-6 weeks; will we do a protocol bump to knock "bad csm" clients out? |
19:02 |
paramat |
maybe |
19:02 |
VanessaE |
ok. |
19:02 |
paramat |
actually yes |
19:03 |
paramat |
because then the restrictions can be enforced |
19:03 |
VanessaE |
make "strict checking" the default then |
19:03 |
VanessaE |
(it's off by default, now, I believe) |
19:03 |
paramat |
of course :] |
19:04 |
VanessaE |
just making sure :) |
19:04 |
paramat |
you know how ifeel about the issue :] |
19:04 |
VanessaE |
perhaps ;) |
19:05 |
red-001 |
VanessaE, then you lose a lot of players |
19:05 |
VanessaE |
oh well. |
19:05 |
red-001 |
look at the server list stats to see what I mean |
19:05 |
VanessaE |
there'll be an official release when that happens. users can upgrade. |
19:06 |
VanessaE |
fwiw, right now daconcepts.com 30001, my dreambuilder survival server, is at the top of the list. |
19:06 |
red-001 |
https://kitsunemimi.pw/tmp/serverlist_stats_2017-03-17.txt |
19:06 |
VanessaE |
that one doesn't work well on mobiles, meaning PC users mostly, and those folks can upgrade easily. |
19:06 |
red-001 |
only 05.89% where latest |
19:06 |
red-001 |
were* |
19:06 |
VanessaE |
right |
19:06 |
paramat |
red, it's optional to prevent old clients, but necessary to completely enforce csm restrictions, as you explained |
19:06 |
paramat |
only some servers will |
19:07 |
VanessaE |
red-001: but then again, look at the stats closer: the majority are on Multicraft. |
19:07 |
VanessaE |
a mobile client |
19:07 |
red-001 |
true |
19:07 |
VanessaE |
so f--- them. :P |
19:07 |
paramat |
hehe |
19:07 |
red-001 |
for minetest clients it was even worse |
19:07 |
red-001 |
most were 0.4.13 |
19:08 |
VanessaE |
red-001: think of it this way: how would 0.5.x affect them, when servers start moving to that? |
19:08 |
Krock |
it's not like they can't update minetest. strict protocol checking will just speed up the process |
19:08 |
VanessaE |
answer of course is that everyone will break for a while |
19:08 |
red-001 |
I do have to ask is 0.5.0 going to fully break backwards compatibility? |
19:08 |
paramat |
yes this will give clients a reason to upgrade, so more will |
19:09 |
VanessaE |
it will break backward compat enough to make it a problem for luddites, yes |
19:09 |
VanessaE |
it already breaks backward compat with older OS's |
19:09 |
paramat |
sorry red, that 'yes' was not for your question |
19:09 |
Krock |
[limited to protocol backwards compability] |
19:09 |
VanessaE |
(because of C++11 requirements) |
19:13 |
paramat |
nore ShadowNinja what are your opinions on game#1543 ? |
19:13 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/1543 -- Add 'creative' privilege for survival servers by tenplus1 |
19:14 |
ShadowNinja |
paramat: Will conflict with unified_inventory. |
19:15 |
paramat |
ok thanks, please add a comment |
19:15 |
ShadowNinja |
(or at least, they both have the same priv, I don't know if Minetest allows redefinitions). |
19:16 |
paramat |
ah so a new priv name is needed? |
19:16 |
nerzhul |
red-001, we did this for breaking compat, and i hope it will be strictly done, we have some protocols problems, in packets for example we have some bad integer truncates or sign mismatch |
19:17 |
nerzhul |
C++11 is just a compiler break, no functional change atm |
19:17 |
ShadowNinja |
paramat: Looks like there are no conflict checks, so UI will just "win" and get to set the priv def. |
19:17 |
paramat |
yes it's a good time to break compat, there's no better time |
19:17 |
Krock |
builtin/game/privileges.lua: core.registered_privileges[name] = def |
19:17 |
Krock |
redefinitions are no problem |
19:18 |
paramat |
but a new priv name is better surely? |
19:19 |
paramat |
i'll merge #5908 later |
19:19 |
ShadowBot |
https://github.com/minetest/minetest/issues/5908 -- (Re)spawn players within 'mapgen_limit' by paramat |
19:19 |
Krock |
I don't see any problem there. This change shouldn't conflict with u_i |
19:20 |
ShadowNinja |
paramat: No, it's fine. |
19:21 |
paramat |
ok you +1? |
19:21 |
paramat |
concept approval |
19:21 |
ShadowNinja |
paramat: Yep. |
19:21 |
paramat |
ok |
19:22 |
|
AntumDeluge joined #minetest-dev |
19:24 |
paramat |
Krock +1? |
19:28 |
Krock |
concept approval from my side too |
19:28 |
paramat |
ok |
19:29 |
paramat |
it can certainly be merged then |
19:32 |
paramat |
game#1758 ? |
19:32 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/1758 -- Stairs: Use one recipe matching inventory appearence by paramat |
19:33 |
paramat |
game#1764 is potentially good now |
19:33 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/1764 -- Adding backface culling to stairs by MrRar |
19:35 |
DS-minetest |
the extra variable in game1764 is unneeded |
19:36 |
paramat |
which variable? |
19:36 |
DS-minetest |
stair_images |
19:37 |
DS-minetest |
`images` can be overridden directly in the loop |
19:37 |
paramat |
ok |
19:38 |
paramat |
i see |
19:39 |
DS-minetest |
and what is if backface_culling (or something else) is already given? imo it shouldn't override then (eg. for glass sairs) |
19:41 |
paramat |
yeah |
19:45 |
paramat |
thanks |
19:47 |
ShadowNinja |
nerzhul: Can you take a look at this?: http://sprunge.us/BiQh?diff It removes threads.h in favor of the C++11 types and methods, and it fixes the threadProc signature Since C++11 supports arbitrary thread function signatures. |
19:50 |
sfan5 |
you should make a pr for that as it changes quite many lines |
19:51 |
|
Agent07 joined #minetest-dev |
19:52 |
ShadowNinja |
sfan5: My PRs usually just sit in the queue for years... It's a good number of lines, but it's mostly just search and replace, so you can go over it quickly. |
19:53 |
ShadowNinja |
It doesn't actually change any functionality too, so as long as it compiles you can be pretty sure it's correct. |
20:28 |
|
YuGiOhJCJ joined #minetest-dev |
20:36 |
nerzhul |
ShadowNinja, if you can do a PR i can review |
20:36 |
ShadowNinja |
nerzhul: Can you just review the patch? |
20:38 |
nerzhul |
too huge i prefer GH |
20:38 |
nerzhul |
i notice you don't have the macro for COPY removal, you have only copy constructor removal not copy assigment removal |
20:40 |
ShadowNinja |
nerzhul: Ehm, I think the C++11 one does both -- but I didn't change that I just moved it. That change was from your previous C++11 threading PR. |
20:41 |
nerzhul |
no it doesn't do both, in all docs i read about this usage both are required |
20:42 |
nerzhul |
just use DISABLE_CLASS_COPY(class) after constructor it will generate it for you |
20:42 |
nerzhul |
and provide a GH pr i will review & test, maybe someone else and we will merge it soon |
20:42 |
nerzhul |
ShadowNinja, oh okay :) then i missed it, but it should be re-used |
20:43 |
nerzhul |
i just want to finish those PR on C++11 base for our code |
20:44 |
nerzhul |
we didn't discussed about #pragma once usage, too late i think, i and zeno was in favour (i should go too), don't hesitate to debate and add a rule if accepted :p |
20:44 |
nerzhul |
i hope we can flush the pr queue and be under 160 PR :( |
20:51 |
ShadowNinja |
I've also created a new .clang-format, which more closely matches the current style. |
20:54 |
ShadowNinja |
!tell nerzhul https://github.com/minetest/minetest/pull/5957 |
20:54 |
ShadowBot |
ShadowNinja: O.K. |
20:57 |
|
Megaf joined #minetest-dev |
21:03 |
|
diegom joined #minetest-dev |
21:06 |
|
Gundul joined #minetest-dev |
21:16 |
|
CalebDavis joined #minetest-dev |
21:31 |
red-001 |
so #5852 can be merged? |
21:31 |
ShadowBot |
https://github.com/minetest/minetest/issues/5852 -- Improve the path select GUI by red-001 |
21:38 |
ShadowNinja |
Hmmm, it seems that if you set clang-format to not add a newline between class Foo and the starting brace, it will also trim all the empty newlines before the class. |
21:38 |
ShadowNinja |
Same for struct, enum, etc. |
21:40 |
ShadowNinja |
Maybe it's a bug or there's some config option for that (although it doesn't look like it). If not, we might have to switch to AStyle or something to get this to work. |
21:43 |
ShadowNinja |
clang-format 4.0 does tha same thing, so probably not a bug (or at least not one likely to be fixed). |
21:46 |
ShadowNinja |
Doesn't look like clang-format supports "class Foo {" without removing all empty lines befpre the befinition. |
21:46 |
ShadowNinja |
AStyle is probably a good choice to replace it. |
21:53 |
|
jin_xi joined #minetest-dev |
22:13 |
|
DI3HARD139 joined #minetest-dev |
22:19 |
|
paramat joined #minetest-dev |
22:33 |
|
halt_ joined #minetest-dev |
22:35 |
red-001 |
halt_ seems to have a bad connection |
23:09 |
|
johnnyjoy joined #minetest-dev |
23:34 |
red-001 |
does CSM need access to the jit table? |
23:34 |
red-001 |
the fucntions in it don't seem like something a mod needs |
23:34 |
red-001 |
and removing it will get rid of one more attack vector |
23:36 |
paramat |
halt/grandolf should probably be banned here, they are in minetest channel |
23:37 |
red-001 |
well they seems to quickly part whenever they join |
23:37 |
paramat |
or at least grandolf is, but apparently they share a computer |
23:37 |
red-001 |
CSM needs a serious security review |
23:37 |
paramat |
they can read logs, there's no need for them to participate here :] |
23:40 |
paramat |
rubenwardy ShadowNinja please could you ban halt_ ? |
23:41 |
ShadowNinja |
paramat: Er, from where? |
23:41 |
red-001 |
ShadowNinja, since you orginaly implemented mod security could you tell me if #5941 correctly implements the isClient function? |
23:41 |
ShadowBot |
https://github.com/minetest/minetest/issues/5941 -- [CSM] allow the client to access and modify settings in the `client.` namespace. by red-001 |
23:42 |
red-001 |
I don't want mods to be able to modify the vaule of isClient since I use it to check if we need to enable more strict sandboxing for settings |
23:42 |
ShadowNinja |
red-001: The server security code isn't supposed to be used on the client in the first place... |
23:44 |
ShadowNinja |
red-001: You don't initialize the RIDX for the server. |
23:44 |
ShadowNinja |
red-001: Also, adding a second seperate "secure" settings namespace seems like a bad idea. |
23:46 |
red-001 |
huh so it's not certain that it will be nil/false if I don't initialize it? |
23:46 |
ShadowNinja |
red-001: Your new security check is flawed -- it will work in all settings files instead of just for the main settings. |
23:47 |
ShadowNinja |
Well -- yes, it would be nil. |
23:47 |
red-001 |
well I don't think CSM is likely going to be allowed to load other setting files |
23:48 |
red-001 |
since it's not meant to have filesystem access |
23:48 |
ShadowNinja |
red-001: You erase the first 7 chars of every setting name in to_table? |
23:48 |
red-001 |
yes |
23:48 |
red-001 |
but only if they are "client." |
23:49 |
red-001 |
it's so that mods don't have to deal with the abstraction of the "client." namespace |
23:49 |
ShadowNinja |
Er, s/to_table/get_names/ |
23:49 |
red-001 |
both? |
23:50 |
sfan5 |
what happens if i name a setting client.secure.something |
23:51 |
ShadowNinja |
Oh, I see what you're doing. I don't like it at all. The security check macro is actually mutating local variables. |
23:51 |
red-001 |
you have a bad naming scheme then |
23:52 |
red-001 |
so should I move it into a function then? |
23:52 |
sfan5 |
it's not about bad naming scheme, its about whether this could confuse mods or result in a security issue |
23:52 |
ShadowNinja |
red-001: You're also not doing the conversion back in get. |
23:52 |
ShadowNinja |
red-001: I think setting groups are just what you should be using here. |
23:53 |
red-001 |
setting groups? |
23:53 |
ShadowNinja |
red-001: https://github.com/minetest/minetest/blob/master/src/settings.h#L137 |
23:54 |
ShadowNinja |
Then do something along the lines of core.settings = LuaSettings(g_settings->getGroup("client")) |