Time |
Nick |
Message |
00:27 |
hmmmm |
https://github.com/kwolekr/minetest/commit/46a2c1f167f76b1ceb0164e9028b67eb6bf76e79 |
01:10 |
|
Wayward_Tab joined #minetest-dev |
01:15 |
|
mint joined #minetest-dev |
01:16 |
|
Wayward_One joined #minetest-dev |
01:33 |
|
devmarth joined #minetest-dev |
01:34 |
devmarth |
https://github.com/blokel/blokel/issues/2 |
01:34 |
devmarth |
Someone take a look at this? The error happened after messing around with camera.cpp to make the default camera view third person, but now even after switching it back, it still gives the error |
02:05 |
|
Wayward_Tab joined #minetest-dev |
02:21 |
hmmmm |
devmarth: Sorry blockel is not supported here |
02:21 |
devmarth |
Lol, but the error would lead to the same place if it happened on Minetest right? hmmmm |
02:22 |
hmmmm |
I dunno about that |
02:23 |
hmmmm |
If you suspect there's an issue with Minetest, please post an entry on the Issues tracker on our GitHub page, at https://github.com/minetest/minetest |
02:23 |
hmmmm |
Please post a complete description of the problem you are experiencing and steps to reproduce. |
02:25 |
|
hmmmmm joined #minetest-dev |
02:38 |
|
Hijiri joined #minetest-dev |
03:00 |
|
hmmmmmm joined #minetest-dev |
03:09 |
|
Zeno` joined #minetest-dev |
03:23 |
Zeno` |
hmmmm has two extra 'm's |
03:23 |
hmmmmmm |
that means I disconnected twice |
03:24 |
Zeno` |
:D |
03:25 |
|
hmmmmmm joined #minetest-dev |
03:25 |
|
hmmmm joined #minetest-dev |
03:42 |
|
Hunterz joined #minetest-dev |
03:51 |
|
paramat joined #minetest-dev |
03:54 |
paramat |
hmmmm this is bemusing #2639 |
03:54 |
ShadowBot |
https://github.com/minetest/minetest/issues/2639 -- The mysterious patchy forests of the biome API |
04:23 |
|
cib0 joined #minetest-dev |
04:39 |
paramat |
aww yeah! solved, it was 'place centre x' 'place centre z', without these the bug is fixed |
04:40 |
paramat |
something to do with those displacing the node check form the heightmap |
04:40 |
paramat |
(from) |
04:40 |
paramat |
which is why the bug is not present on a flat landscape =) |
04:52 |
paramat |
it came to me while doing the washing-up |
04:55 |
|
hmmmm joined #minetest-dev |
04:55 |
hmmmm |
heh |
04:55 |
hmmmm |
it's probably time to pay down some technical debt |
04:56 |
hmmmm |
i dunno about you guys but right now i'm working on noise unit tests and schematic unit tests |
04:56 |
paramat |
i'll work on a fix |
04:56 |
hmmmm |
it might not be so easy to fix, lemme check |
04:57 |
paramat |
yeah, what i mean is 'attempt' |
04:58 |
paramat |
my understanding is now commented in the thread |
04:59 |
hmmmm |
so yeah, where a decoration is placed gets determined in Decoration::placeDeco, but the flags get applied in DecoSchematic::generate() |
04:59 |
hmmmm |
i sort of expected a problem like this but wasn't exactly sure what |
04:59 |
hmmmm |
i've dealt with this sort of thing before but apparently not all the cases |
05:00 |
hmmmm |
so right now there are only two types of decorations, simple and schematic |
05:00 |
hmmmm |
simple is a single vertical node column so it doesn't have this problem |
05:01 |
hmmmm |
but when you think of all the future decorations, like lsystem trees and structures, they both have the same problem in common with schematics |
05:02 |
hmmmm |
the abstractions currently supplied by Decoration are not sufficient |
05:02 |
paramat |
the place_on checks are before flags are applied so the code order seems correct |
05:04 |
paramat |
actually the order is not correct |
05:05 |
paramat |
place_on check comes after flags are applied |
05:05 |
hmmmm |
how is that an incorrect order |
05:05 |
paramat |
(for schematics) |
05:06 |
paramat |
position for place_on check is displaced sideways, therefore under/overground |
05:08 |
paramat |
canPlaceDecoration should be used before flags apply 'place centre x/z' |
05:08 |
paramat |
well shall i try it and see? |
05:09 |
paramat |
rather, use if (!CONTAINS(c_place_on, c)) return 0; before flags applied |
05:10 |
hmmmm |
wait, i'm not convinced |
05:11 |
hmmmm |
how does the order of c_place_on matter? |
05:11 |
hmmmm |
hrmm |
05:11 |
hmmmm |
oh, it's checking the wrong thing |
05:11 |
hmmmm |
we want to make sure the center of the tree, i.e. the trunk, is being placed on the correct thing |
05:12 |
hmmmm |
so the pos we decided on earlier is where the center is going to be placed |
05:12 |
paramat |
it is |
05:12 |
hmmmm |
indeed, that's the problem. great job =] |
05:12 |
paramat |
shall i fix? |
05:13 |
hmmmm |
in any case, schematics are far from perfect. it's incredible how much attention they get and how much work they still need |
05:13 |
hmmmm |
yes, while you're there however, take a look at vm->m_area.contains(p) |
05:13 |
hmmmm |
that's wrong for schematics obviously |
05:14 |
hmmmm |
it happens to work because blitToVManip() is careful and checks if each point is within the voxelmanip, but this could cause cutoffs |
05:14 |
paramat |
ah should be limited to the mapchunk only |
05:14 |
paramat |
? |
05:14 |
hmmmm |
no |
05:14 |
paramat |
check if edges of schem are within the vm |
05:15 |
hmmmm |
do contains() on the VoxelArea that the schematic is going to take up, not the precise point of the corner of the schematic |
05:15 |
hmmmm |
contains() is overloaded and takes both a v3s16 and a VoxelArea & |
05:15 |
hmmmm |
like i said. it's far from perfect. |
05:16 |
hmmmm |
and now add rotations into the mix =] |
05:16 |
paramat |
ugh |
05:16 |
hmmmm |
then sokomine will come back in a couple days |
05:16 |
hmmmm |
"hey this doesn't work with the specific combination of PLACE_CENTER_X and rotation for a facedir > 4! |
05:17 |
hmmmm |
"but I never intended full rotations..." |
05:17 |
hmmmm |
"oh also please hmmm could you add mirroring for schematics" |
05:17 |
paramat |
(hehe) well i might tackle the rotations > 3 thing |
05:17 |
hmmmm |
don't bother |
05:17 |
hmmmm |
schematics were only designed for rotations along the Y axis |
05:17 |
hmmmm |
it's way too much work for not much return |
05:17 |
paramat |
okay |
05:18 |
hmmmm |
anyway |
05:19 |
hmmmm |
add a helper function like getRotatedSize() or something and check against that while still in DecoSchematic::generate() before applying center transforms |
05:19 |
hmmmm |
that should do the trick for the edge case i mentioned earlier |
05:20 |
hmmmm |
"<Sokomine in PM> hmmmm thank you so much for your work on schematics :) now can you add a lawn mowing function? i really need it for my villages mod and it would make it soo much better" |
05:20 |
hmmmm |
lol |
05:20 |
hmmmm |
:( |
05:22 |
paramat |
:P well i can do the first 2 things.. to work |
05:22 |
hmmmm |
it's not worth it to satisfy every single edge case possible |
05:22 |
hmmmm |
this is what people do and they end up spending all their energy on it |
05:23 |
paramat |
"just do something simple for now" |
05:24 |
|
OldCoder joined #minetest-dev |
05:24 |
hmmmm |
"a working complex system invariably once started out as a working, simple system" |
05:25 |
hmmmm |
if you try to plan too much from the get go it's going to be unsuccessful |
05:31 |
paramat |
how about using canPlaceDecoration for decoschems also so they can use 'spawnby'? |
05:31 |
hmmmm |
there was a reason why i didn't do that |
05:32 |
paramat |
okay |
05:32 |
hmmmm |
i forget right now why |
05:32 |
paramat |
well it must have been a good reason |
05:32 |
hmmmm |
oh right |
05:33 |
hmmmm |
one of the reasons was because spawn_by makes no sense for schematics |
05:33 |
paramat |
oh of course |
05:33 |
hmmmm |
unless it's a vertical column |
05:33 |
hmmmm |
lol |
05:34 |
hmmmm |
schematics can have the same thing but it'd require special handling and it'd be a very computationally intensive check |
05:34 |
paramat |
i won't for now |
05:34 |
hmmmm |
i.e. you'd need to check the entire plane of nodes below the schematic |
05:35 |
hmmmm |
for tress I suppose this doesn't make much sense because the stem is a single column, but how are you supposed to generalize that? |
05:35 |
hmmmm |
trees* |
05:36 |
paramat |
it wuld just be a rough check for spawnby nodes surrounding the schematic centre, possibly still useful |
05:36 |
paramat |
(would) |
05:37 |
|
Player_2 joined #minetest-dev |
05:37 |
hmmmm |
so maybe it can be generalized to all schematics, with the caveat that it only checks the neighbors of a *single* node at the exact spot where the translated position is expected |
05:38 |
hmmmm |
and then schematics specifically gets a heavy, complete version that checks all nodes near the bottom plane |
05:38 |
sfan5 |
<neoascetic> Why Uppercase? |
05:38 |
sfan5 |
<neoascetic> And then PROJECT_NAME_LOWER almost everywhere |
05:38 |
sfan5 |
<Zeno`> It should be lowercase |
05:38 |
sfan5 |
<Zeno`> uppercase is just... silly :) |
05:38 |
paramat |
nah i wouldn't bother, just check spawnby for nodes around the schem centre |
05:39 |
paramat |
useful for trees |
05:39 |
sfan5 |
I told ShadowNinja that this uppercase/lowercase stuff is stupid |
05:39 |
sfan5 |
there was a comment on the PR saying that |
05:39 |
sfan5 |
but apparently nobody read the PR before it was merged |
05:40 |
hmmmm |
it's easy to fix though, just a simple find and replace pass |
05:40 |
hmmmm |
I really wish people would focus on better documentation or unit testing instead of endless refactors |
05:45 |
Zeno` |
? |
05:45 |
Zeno` |
oh, yeah. I don't think it should be uppercase |
05:47 |
|
Hunterz joined #minetest-dev |
05:51 |
|
paramat left #minetest-dev |
06:05 |
|
selat joined #minetest-dev |
06:21 |
|
neoascetic joined #minetest-dev |
06:22 |
neoascetic |
hi all. what are "leveled" nodes? (minetest.add_node_level etc) |
06:23 |
neoascetic |
ShadowNinja https://github.com/neoascetic/minetest/blame/master/CMakeLists.txt#L9-L10 doesn't "minetest" should be lowercased, like on github? |
06:24 |
hmmmm |
neoaesthetic |
06:24 |
hmmmm |
I ended up not being able to get minetest built |
06:25 |
hmmmm |
how did you compile minetest on OS X? irrlicht fails because it can't find opengl/gl.h, for me at least |
06:26 |
neoascetic |
dunno why u have a problems |
06:27 |
hmmmm |
i really want to get this fixed (and in a non-hacky way) |
06:27 |
neoascetic |
https://github.com/neoascetic/minetest/blob/builds/.travis.yml#L4-L12 |
06:27 |
neoascetic |
I am doing these steps usually |
06:27 |
hmmmm |
ahh okay i'll try brew |
06:28 |
hmmmm |
so your travis build works? |
06:28 |
neoascetic |
yep |
06:28 |
hmmmm |
maybe we could somehow get that integrated into our travis ci thing |
06:28 |
hmmmm |
i don't like testing only 3 of the 4 major platforms |
06:28 |
neoascetic |
we talked with est31 about this previously |
06:29 |
neoascetic |
yesterday. with travis, it is kind of hacky and semi-legal way |
06:29 |
neoascetic |
but possible |
06:29 |
hmmmm |
bleh |
06:29 |
neoascetic |
did you read this? https://github.com/minetest/minetest/pull/1928#issuecomment-66501792 |
06:29 |
hmmmm |
using mac os x is definitely not illegal, i hate it when people say that |
06:30 |
neoascetic |
I am talking about travis integration |
06:30 |
neoascetic |
they are out of Macs capacity. We don't hurt travis, do we? |
06:30 |
neoascetic |
don't want to * |
06:31 |
hmmmm |
it has something to do with "being a cocoa application" |
06:31 |
hmmmm |
I'm sure there's a better way to do it than placing the file in a specific location |
06:31 |
hmmmm |
like an api call or something |
06:31 |
neoascetic |
yeh, I've tried to search yesterday, but with no success |
06:31 |
hmmmm |
but as it exists right now, it breaks the entire point of RUN_IN_PLACE |
06:31 |
neoascetic |
all links on irrlicht forums point to this workaound: build a .app |
06:32 |
hmmmm |
when I figure it out I'll be sure to post about it on the forums |
06:32 |
neoascetic |
the easiest way to solve this is to create Contents directory with build system if RUN_IN_PLACE == 1 |
06:32 |
neoascetic |
:) |
06:33 |
neoascetic |
ok, I hope you will find a solution |
06:33 |
neoascetic |
however, RUN_IN_PLACE is not a big issue for end user |
06:33 |
hmmmm |
how do you get 'brew'? |
06:33 |
hmmmm |
ah nevermind i have to insall it manually |
06:33 |
neoascetic |
http://brew.sh/ |
06:37 |
|
jin_xi joined #minetest-dev |
06:42 |
hmmmm |
does brew not support Lion or something? |
06:43 |
neoascetic |
probably |
06:43 |
neoascetic |
do you have Lion? |
06:44 |
hmmmm |
yes |
06:44 |
hmmmm |
I don't suppose there's any way to automatically update to the latest |
06:45 |
neoascetic |
I think brew should be ok with Lion |
06:45 |
neoascetic |
do you have an error? |
06:45 |
hmmmm |
yes |
06:45 |
hmmmm |
hrmm |
06:45 |
hmmmm |
I can't copy and paste... |
06:46 |
neoascetic |
use the mouse :) |
06:46 |
hmmmm |
no it's vmware |
06:47 |
hmmmm |
err virtualbox |
06:47 |
neoascetic |
guest additions or something... |
06:48 |
hmmmm |
doesn't work for osx |
06:48 |
hmmmm |
http://www.fpaste.org/213675/29598893/ |
06:48 |
hmmmm |
/usr/bin/install_name_tool: object: /usr/local/Cellar/jpeg/8d/lib/libjpeg.8.dylib malformed object (unknown load command 9) |
06:49 |
neoascetic |
2-5 lines |
06:49 |
hmmmm |
tells me to get xcode 5.1.1 but i can't install anything more recent than 4.6 |
06:49 |
neoascetic |
you don't have to install entier xcode, just developer command lines tools or something like this |
06:52 |
hmmmm |
i'm downloading mavericks |
06:52 |
hmmmm |
this is gonna take a while |
07:45 |
|
neoascetic joined #minetest-dev |
07:47 |
neoascetic |
hmmmm could you please download yosemite instead? |
08:01 |
|
Yepoleb_ joined #minetest-dev |
08:06 |
neoascetic |
Not sure if #2627 need to be merged. I have no problem with compiling minetest on Yosemite |
08:06 |
ShadowBot |
https://github.com/minetest/minetest/issues/2627 -- Fix endian.h compile error on OSX and use native byte-swap functions on BSD by snowyu |
08:07 |
neoascetic |
FreeBSD also works, as per CI report |
08:10 |
neoascetic |
Altrough native byte-swap function should be a good idea |
08:18 |
|
Darcidride joined #minetest-dev |
08:30 |
|
blaze joined #minetest-dev |
08:36 |
|
Hunterz joined #minetest-dev |
09:06 |
|
cib0 joined #minetest-dev |
09:12 |
OldCoder |
Server list is built into core, correct? How would I apply to become a server list host? |
09:13 |
OldCoder |
Rather, list of server lists is built into core |
09:13 |
neoascetic |
OldCoder nope |
09:14 |
neoascetic |
https://github.com/minetest/minetest/blob/master/minetest.conf.example#L318-L322 |
09:15 |
OldCoder |
neoascetic, there is a default server list |
09:15 |
OldCoder |
Correct? |
09:15 |
OldCoder |
You are linking I assume to the announce settings |
09:15 |
OldCoder |
I am referring to *being* the server list, not to being *on* it; do you follow? |
09:16 |
neoascetic |
serverlist_url |
09:16 |
OldCoder |
Yes; and there is a default, correct? |
09:16 |
neoascetic |
yep |
09:16 |
OldCoder |
It is said that the current default server list server is bad |
09:16 |
OldCoder |
I have run an octocore for several years now with numerous worlds |
09:16 |
neoascetic |
improve it, code is on github |
09:17 |
OldCoder |
Are the problems in the code? |
09:17 |
OldCoder |
Or external? |
09:17 |
neoascetic |
you said is bad |
09:17 |
OldCoder |
I am told this |
09:17 |
OldCoder |
At what level are the problems coming from? |
09:18 |
neoascetic |
I don't know what problems you talking about |
09:18 |
OldCoder |
And, regardless, is there a strong case for retaining the current server list server? Or for limiting it to one server? |
09:18 |
OldCoder |
Vanessa and others have told me that the list is hosed |
09:18 |
OldCoder |
That it does not work reliably |
09:18 |
OldCoder |
If this is true, and if the problems are external, I will apply to host the list |
09:19 |
OldCoder |
It can hardly be argued that I and my hardware are not qualified |
09:20 |
|
Megaf joined #minetest-dev |
09:21 |
neoascetic |
My idea is use heroku or something instead |
09:28 |
OldCoder |
Heroku? O_o |
09:29 |
|
neoascetic joined #minetest-dev |
09:37 |
celeron55 |
OldCoder: well i guess you now applied to become a server list host; however if people think there is a problem with the current server list's hosting, report to sfan5 (the current host) and me (because i manage the DNS) so that problems can be looked at |
09:38 |
celeron55 |
i haven't really heard of such issues so i assume there is no need for any changes |
10:10 |
celeron55 |
...inspired by that, i now drafted this: http://www.minetest.net/reporting_issues |
10:10 |
celeron55 |
comments? |
10:10 |
celeron55 |
or additions? |
11:35 |
|
paramat joined #minetest-dev |
11:36 |
paramat |
hmmmm #2640 here's the fix for issue 2639 |
11:36 |
ShadowBot |
https://github.com/minetest/minetest/issues/2640 -- DecoSchematic: Fix missing trees in rough terrain by paramat |
11:36 |
|
paramat left #minetest-dev |
11:38 |
|
OldCoder joined #minetest-dev |
11:41 |
OldCoder |
celeron55, thank you; I am falling asleep but will review |
11:44 |
|
Wayward_Tab joined #minetest-dev |
11:48 |
|
err404 joined #minetest-dev |
11:49 |
sfan5 |
<OldCoder> That it does not work reliably |
11:50 |
sfan5 |
i didn't hear of any problems with the server list either |
11:50 |
OldCoder |
sfan5, all right; I was under the impression this was well known |
11:50 |
OldCoder |
Will seek specifics |
11:50 |
OldCoder |
BTW Meowtest continues |
11:50 |
sfan5 |
if there are problems it would make sense to tell me because i host the server list |
11:50 |
sfan5 |
thats good |
11:50 |
OldCoder |
Thank you for the information |
11:51 |
* OldCoder |
is half asleep |
12:10 |
|
OldCoder joined #minetest-dev |
12:52 |
|
ElectronLibre joined #minetest-dev |
13:01 |
|
chchjesus joined #minetest-dev |
13:20 |
|
est31 joined #minetest-dev |
13:22 |
est31 |
celeron55, fine site. I'll ask fdroid people to set it as the "report bugs here" url for minetest app. |
13:23 |
est31 |
and Oldcoder, I guess there is a technical limitation: If we want multiple servers, we will need a mechanism for serverlist servers to share information about new servers, so that both serverlists contain the same servers |
13:24 |
est31 |
one new minetest server will only report to one of the servers. |
14:13 |
|
kilbith joined #minetest-dev |
14:15 |
|
Zeno` joined #minetest-dev |
14:22 |
|
hmmmm joined #minetest-dev |
14:28 |
ShadowNinja |
~tell neoascetic No, it's supposed to be cased like that because It's used like that in some places, it's more complicated to convert a lowercase version to mixed case, and that's its proper capitaliation acording to english style rules because it's a proper noun. PROJECT_NAME_LOWER is used largely for compatability reasons. It should probably be passed as a macro to avoid the runtime lowercase calls though. |
14:28 |
ShadowBot |
ShadowNinja: O.K. |
14:30 |
sfan5 |
" it's more complicated to convert a lowercase version to mixed case" not really |
14:30 |
sfan5 |
"and that's its proper capitaliation acording to english style rules because it's a proper noun." |
14:30 |
sfan5 |
I guess thats why every other thing (suck as package names) are uppercase too |
14:30 |
sfan5 |
(hint: they're not) |
14:31 |
|
AnotherBrick joined #minetest-dev |
14:31 |
ShadowNinja |
sfan5: Well, if you keep the current casing, but say a fork used casing like MineTest... |
14:31 |
sfan5 |
then it's the problem of the fork |
14:31 |
sfan5 |
we can't do stuff to solve problems that might happen if someone does something we did not forsee |
14:32 |
* Zeno` |
emails Emacs because their package name and binary name is... emacs |
14:32 |
ShadowNinja |
sfan5: Which they have to solve with a custom lower-to-upper function wich changes chars at specific offsets, instrad of just setting the name and having it automatically converted for everything. |
14:32 |
Zeno` |
idiots don't know proper English |
14:32 |
ShadowNinja |
sfan5: I *am* forseeing this! |
14:33 |
sfan5 |
ShadowNinja: a fork might also want to change the name to something with utf-8 |
14:33 |
sfan5 |
where's the code for that? |
14:33 |
ShadowNinja |
sfan5: Do I need to add some? |
14:33 |
sfan5 |
you don't get it |
14:34 |
sfan5 |
We CAN'T add code for every possible thing someone might do with PROJECT_NAME in the future in a fork |
14:34 |
ShadowNinja |
The string lowering functions mught not work on non-latin characters, but other than that it should work. |
14:34 |
sfan5 |
uh |
14:34 |
sfan5 |
isn't the project name put into a std::string |
14:34 |
sfan5 |
like |
14:34 |
ShadowNinja |
Yep. |
14:34 |
sfan5 |
we do have all that wide_to_narrow stuff for a reason, do we? |
14:34 |
ShadowNinja |
wide_to_narrow is for Irrlicht. |
14:35 |
ShadowNinja |
Everything else can use UTF-8. |
14:35 |
sfan5 |
isn't the project name passed to irrlicht? |
14:35 |
ShadowNinja |
Yes. |
14:35 |
Zeno` |
well... it's not in my SDL fork |
14:35 |
Zeno` |
but I guess that's another story |
14:36 |
sfan5 |
anyway |
14:36 |
ShadowNinja |
Zeno`: You got Minetest to work under SDL? |
14:36 |
sfan5 |
this isn't the point |
14:37 |
sfan5 |
ShadowNinja: irrlicht has an sdl backend |
14:37 |
ShadowNinja |
Ah, O.K. |
14:37 |
sfan5 |
the point is that we can't add stuff for every possible thing someone might do in a fork in the future |
14:37 |
ShadowNinja |
But we can easily do this. |
14:37 |
sfan5 |
that doesn't mean we should |
14:38 |
ShadowNinja |
Why shouldn't we? |
14:38 |
sfan5 |
because it's stupid |
14:39 |
ShadowNinja |
Well, isn't that a nice, well thought out, logical argument for why we should change how we handle this! |
14:39 |
ShadowNinja |
;-) |
14:40 |
sfan5 |
your argument for changing it to uppercase is just as bad |
14:40 |
sfan5 |
someone could want to use GNU/Hurd with Minetest, quick, ass gnu/hurd support |
14:40 |
sfan5 |
add* |
14:42 |
sfan5 |
changing it to uppercase just complicates things |
14:42 |
sfan5 |
there is no (good) reason to change it |
14:42 |
ShadowNinja |
sfan5: Uh, didn't someone already fix Hurd support? |
14:43 |
sfan5 |
yes, it was just an example anyway |
14:43 |
ShadowNinja |
I don't understand how this relates to the casing thing. |
14:44 |
ShadowNinja |
It works well as it is now, and you'd be removing a fork support feature by changing it. |
14:44 |
sfan5 |
" you'd be removing a fork support feature by changing it." |
14:44 |
sfan5 |
huh lol |
14:44 |
sfan5 |
"we added it, not we can't remove it again" |
14:44 |
sfan5 |
now* |
14:45 |
ShadowNinja |
The only thing that I think should be changed is adding a PROJECT_NAME_LOWER macro to the source so that lowercase() doesn't have to be called at runtime. |
14:45 |
sfan5 |
"some fork may want to have a mixed-case name, quick, add support for it to the CMake files and change a lot of stuff" |
14:45 |
sfan5 |
"someone might want to use Minetest with Minix, quick, ass minix support" |
14:45 |
sfan5 |
ShadowNinja: ^ better? |
14:46 |
ShadowNinja |
sfan5: I don't understand what you're trying to argue. That we shouldn't add features unless a lot of people are asking for them? |
14:47 |
sfan5 |
not exactly that but basically yes |
14:51 |
|
err404 joined #minetest-dev |
14:53 |
ShadowNinja |
sfan5: I disagree, and this patch kills and performance difference: http://sprunge.us/XZXR?diff |
14:53 |
sfan5 |
i know that you disagree |
14:54 |
sfan5 |
i don't dislike the change for performance reasons |
14:54 |
ShadowNinja |
sfan5: What do we gain by befaulting to lowercase? |
14:54 |
sfan5 |
nothing |
14:54 |
sfan5 |
except that the zip is no named Minetest-something.zip (which looks stupid) |
14:55 |
sfan5 |
ShadowNinja: what do we gain by defaulting to uppercase? |
14:55 |
sfan5 |
(some fork might want to use it is a bad argument) |
14:55 |
sfan5 |
s/use it/do this and that/ |
14:55 |
ShadowNinja |
sfan5: If you don't like that just change the PROJECT_NAME to PROJECT_NAME_LOWER in the relevant line in CMakeLists.txt |
14:56 |
ShadowNinja |
sfan5: How is that a bad argument? |
14:56 |
sfan5 |
it's the "someone could possibly want this, let's add it" thing i tried to explain |
14:57 |
sfan5 |
it might be a feature (if you even want to call it that) |
14:57 |
sfan5 |
but nobody uses it |
14:57 |
sfan5 |
so it's an useless feature |
14:57 |
sfan5 |
and I don't think that somebody will use it in the near future |
14:59 |
ShadowNinja |
sfan5: It means that we don't have to add it later, and at this point we'd be removing a feature. We could rename NAME to NAME_UPPER if that would make you feel better. |
15:00 |
sfan5 |
i told you the reasons i dislike the change for |
15:00 |
sfan5 |
it's not the variable name and not (exclusively) the folder name |
15:00 |
ShadowNinja |
sfan5: You fine with the PROJET_NAME performance fix I just linked? |
15:00 |
sfan5 |
no |
15:00 |
ShadowNinja |
sfan5: Why not? |
15:01 |
sfan5 |
I disagree with the basic upper-case change |
15:01 |
sfan5 |
adding/changing more code will only make it harder to remove |
15:01 |
sfan5 |
also |
15:01 |
sfan5 |
<ShadowNinja> sfan5: It means that we don't have to add it later |
15:01 |
sfan5 |
we won't have to add it later |
15:02 |
sfan5 |
because it's the forks problem if they want to change something the code currently doesn't support |
15:02 |
Zeno` |
line 49 has a bug... should be checking for sizeof(buf) - 1 (or >=) |
15:03 |
ShadowNinja |
sfan5: This is what you want? http://pastebin.ubuntu.com/10861815/ |
15:03 |
sfan5 |
yes |
15:03 |
sfan5 |
except |
15:04 |
sfan5 |
why do we need the uppercase stuff in the cmake file? |
15:04 |
ShadowNinja |
Because it's used un uppercase! |
15:04 |
ShadowNinja |
Eg with minetest --version and the upper left text. |
15:04 |
Zeno` |
line 49 of http://sprunge.us/XZXR?diff I mean |
15:05 |
sfan5 |
ShadowNinja: we didn't have an uppercase thing in the cmake files before |
15:05 |
ShadowNinja |
sfan5: Yes, because "Minetest" was hardcoded. |
15:06 |
sfan5 |
un-hardcoding that doesn't justify to put it into the cmake files |
15:06 |
ShadowNinja |
... |
15:07 |
ShadowNinja |
Where else should it be? |
15:07 |
sfan5 |
i don't know |
15:08 |
ShadowNinja |
Does that performance fix seem fine now? (that beg Zeno` mentioned was in a context line) |
15:08 |
ShadowNinja |
bug* |
15:09 |
Zeno` |
yeah, I wasn't saying it was related |
15:10 |
Zeno` |
Can someone change the relational operator at https://github.com/minetest/minetest/blob/master/src/porting.cpp#L483 from > to >= please? (I can't right now) |
15:10 |
Zeno` |
err maybe I can edit on github |
15:11 |
Zeno` |
nah, I'd rather not do that |
15:11 |
Zeno` |
too scary |
15:11 |
ShadowNinja |
I'll do it, as long as you're sure this won't break it :-) |
15:12 |
Zeno` |
it won't break anything unless the bug is a feature :) |
15:12 |
Zeno` |
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683188%28v=vs.85%29.aspx |
15:13 |
ShadowNinja |
Hmmm, len == sizeof(buf) seems to be undefined. |
15:14 |
ShadowNinja |
It returns the length needed, which should be more than you passed it. |
15:14 |
ShadowNinja |
Since the length includes the NUL. |
15:14 |
Zeno` |
the length returned, yeah. So it should be >= |
15:15 |
Zeno` |
if sizeof buff == 5 and whatever the env var is is "12345" it will return 5 (meaning there is no NUL and buf was not large enough) |
15:16 |
ShadowNinja |
Zeno`: But len == sizeof(buf) means "the buffer was too small, I need a buffer at least `len` in size to store the full string and the NUL" |
15:16 |
Zeno` |
correct |
15:16 |
ShadowNinja |
But the buf is that size! |
15:16 |
ShadowNinja |
So it should be an impossible return value. |
15:16 |
Zeno` |
no |
15:17 |
Zeno` |
The size of the buffer pointed to by the lpBuffer parameter, including the null-terminating character, in characters. |
15:17 |
ShadowNinja |
It returns 6 if the string's "12345". It includes the NUL. |
15:17 |
Zeno` |
If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer, not including the terminating null character. |
15:18 |
ShadowNinja |
...fails... the return value is the buffer size, in characters, required to hold the string &and its terminating null character* |
15:18 |
Zeno` |
So to me that means "12345" would return 5 |
15:18 |
ShadowNinja |
Zeno`: If the buffer was at least 6 chars long, yes. |
15:19 |
ShadowNinja |
If not it would return 6. |
15:19 |
Zeno` |
hmm |
15:21 |
Zeno` |
ok |
15:22 |
Zeno` |
carry on :) |
15:25 |
ShadowNinja |
Zeno`: You O.K. with my patch? |
15:31 |
Zeno` |
That was a patch? |
15:32 |
ShadowNinja |
Zeno`: Yes, the lowercase(PROJECT_NAME) -> PROJECT_NAME_LOWER |
15:32 |
Zeno` |
I wasn't really paying attention. Umm. Hmm. I don't think it does much :) |
15:33 |
Zeno` |
I mean it does get rid of the call to lowercase, but who cares in that context? :) |
15:34 |
Zeno` |
There is nothing wrong with the patch though, if you want to ask that question :P It's 1:35AM here and I'm not really here |
15:38 |
sfan5 |
ShadowNinja: "(me on github) You probably need to change fewer lines when adding a capitalize function." |
15:38 |
sfan5 |
i just counted through the changes |
15:39 |
sfan5 |
you changed project_name to project_name_lower or project_name to lowercase(project_name) 29 times |
15:39 |
sfan5 |
only to add project_name in some places |
15:41 |
sfan5 |
s/some/16/ |
15:52 |
sfan5 |
ShadowNinja: what about this? https://github.com/minetest/minetest/pull/2641 |
15:57 |
|
leat joined #minetest-dev |
16:15 |
|
SudoAptGetPlay joined #minetest-dev |
16:18 |
|
chchjesus joined #minetest-dev |
16:28 |
hmmmm |
oh shit |
16:28 |
|
Hunterz joined #minetest-dev |
16:28 |
hmmmm |
noise is broken for lacunarity < 0.1 |
16:28 |
hmmmm |
1.0 rather |
16:28 |
hmmmm |
ofactor should = min{L^i} for i=0..n-1 where L is the lacunarity, n is the number of octaves |
16:29 |
hmmmm |
here I mistakenly assume that the maximum value will be the maximum octave number |
16:32 |
|
Hijiri joined #minetest-dev |
16:42 |
|
leat joined #minetest-dev |
16:47 |
|
Robert_Zenz joined #minetest-dev |
17:00 |
hmmmm |
https://github.com/kwolekr/minetest/commit/90a0187ca191b73073db64d71e071fee72aae091 |
17:00 |
|
MinetestForFun joined #minetest-dev |
17:10 |
hmmmm |
pushing... |
17:21 |
|
kilbith joined #minetest-dev |
17:22 |
|
est31 joined #minetest-dev |
17:23 |
|
Krock joined #minetest-dev |
17:39 |
|
SudoAptGetPlay joined #minetest-dev |
18:01 |
|
SudoAptGetPlay joined #minetest-dev |
18:05 |
Krock |
#2642 fixes a bug which appeared with commit 386d695 |
18:05 |
ShadowBot |
https://github.com/minetest/minetest/issues/2642 -- Fix crash on startup (Windows) by SmallJoker |
18:06 |
hmmmm |
why would that cause a crash?? |
18:07 |
hmmmm |
isn't that an indication of a much larger problem? |
18:07 |
Krock |
Bug reported here: https://forum.minetest.net/viewtopic.php?p=176721#p176721 |
18:08 |
Krock |
'/' is not '\\', thus, the function to remove the exeutable name from the path does not work correctly |
18:08 |
|
Calinou joined #minetest-dev |
18:08 |
hmmmm |
how how does that cause the crash |
18:08 |
Krock |
"dir .." on a filepath is not addepted |
18:08 |
Krock |
*accepted |
18:09 |
Krock |
*cd .. |
18:09 |
Krock |
I tested my changes and it solved the problem |
18:10 |
hmmmm |
yes but my point is that if a certain input is causing a crash, that should be fixed as well in addition to producing the correct input |
18:14 |
Krock |
hmmmm, http://stackoverflow.com/questions/875249/how-to-get-current-directory -> the used function "GetModuleFileNameA" returns the path with the executable name |
18:14 |
|
SudoAptGetPlay joined #minetest-dev |
18:15 |
hmmmm |
what? i didn't say anything about that at all |
18:16 |
est31 |
I understand what Krock means. |
18:16 |
Krock |
Okay, now am I just sitting here and dunno about the actual target. |
18:16 |
Krock |
est31, thank you :) |
18:17 |
hmmmm |
can you explain because I don't. GetModuleFileNameA has nothing to do with the application crashing because a certain piece of a string wasn't removed |
18:18 |
est31 |
hmmmm, as it seems, the function getCurrentExecPath returns the path of the executable. |
18:18 |
Krock |
Well, that function is called to get the path. pathRemoveFile(buf, '/'); was previously used to remove the executable name from the string; if there's no '/' inside, there won't be anything cut away -> crash |
18:18 |
est31 |
the windows implementation is done with getCurrentExecPath |
18:19 |
est31 |
and yes what Krock said, the code assumes directory delimiters to be "/" |
18:19 |
est31 |
this isn't portable |
18:19 |
Krock |
exactly. |
18:19 |
hmmmm |
Krock: Then the bug is inside of pathRemoveFile(), which I did not modify at all |
18:20 |
* Krock |
facepalms |
18:20 |
hmmmm |
.... |
18:20 |
hmmmm |
I don't think we're understanding eachother |
18:20 |
hmmmm |
A function crashes because of some kind of invalid input. This is called a bug. |
18:20 |
Krock |
Yeah, same thoughts on here |
18:21 |
|
Robert_Zenz joined #minetest-dev |
18:21 |
est31 |
ERROR[main]: cannot open D:\MineTest\bin\minetest.exe\..\builtin\init.lua: No such file or directory |
18:21 |
Krock |
Maybe I should move the pathRemoveFile() function to the Win32 code of getCurrentExecPath() |
18:21 |
est31 |
^ thats the error |
18:21 |
hmmmm |
I understand completely that looking for '/' instead of the platform-specific char did expose the bug, and this was a trivial oversight that's easily fixed |
18:21 |
Krock |
to make it clear for everybody |
18:21 |
hmmmm |
est31: That isn't a crash |
18:21 |
est31 |
AsyncWorkderThread execution of async base environment failed! |
18:21 |
est31 |
thats the crash |
18:21 |
hmmmm |
that is not a crash. |
18:22 |
hmmmm |
that's an exit after some kind of initialization failure. |
18:22 |
est31 |
ah yes |
18:22 |
Krock |
Oh well, it opens a crash dialog after thowing the Lua errors |
18:22 |
est31 |
for end users it is a crash. |
18:23 |
hmmmm |
Maybe there's some kind of slight language barrier here or something but |
18:23 |
hmmmm |
when you say the word "crash", this means, to me at least, that the application abnormally terminated due to a SIGSEGV, or SIGILL, SIGABRT, or simmilar |
18:23 |
hmmmm |
having an error and then terminating is not a crash. |
18:25 |
hmmmm |
moreover, *if there was a crash* in pathRemoveFile because the character to remove wasn't found in the string, the correct response would be to both a). fix the root cause of the crash, and b). fix the erroneous circumstances creating the edge case that exposed the crash to begin with |
18:25 |
Krock |
I think that's counted as a crash: http://i.imgur.com/fRhQf8P.png |
18:25 |
hmmmm |
that's happening elsewhere |
18:26 |
hmmmm |
it looks to me like it could be an unhandled exception causing a SIGABRT |
18:27 |
Krock |
sorry, I'm not familiar with SIG* terminations |
18:27 |
est31 |
I think you mean its like #2636. I could fix it with a single line commit, but there is a deeper root cause which that "fix" would leave unchanged. |
18:27 |
ShadowBot |
https://github.com/minetest/minetest/issues/2636 -- Verbose logging bloated regression |
18:28 |
est31 |
what* |
18:28 |
hmmmm |
yes, a deeper root cause |
18:29 |
hmmmm |
either patching over the symptoms of a bug or correcting the input that caused a crash to occur but not doing both is not an appropriate fix |
18:29 |
hmmmm |
of course, this is within reason |
18:30 |
hmmmm |
if you had defined a function stringcopy(char *s1, char *s2) { while (*s1++ = *s2++); }, I don't think anybody would fault stringcopy() for defending against NULL parameters. |
18:31 |
hmmmm |
however, I would argue that it's VERY reasonable to expect pathRemoveFile() to not crash or cause some kind of undefined behavior if the character wasn't in the string |
18:31 |
hmmmm |
the former makes an assumption about preconditions; the latter would be shoddy coding |
18:32 |
Krock |
If I may ask, what would you do in this case? |
18:34 |
hmmmm |
simply not modify the string it's supposed to be modifying. |
18:34 |
hmmmm |
that's what it actually does. in my last paragraph, I was only speaking rhetorically. now that I have the additional information that the crash was actually a program abort due to an initialization failure, this makes your fix totally sufficient |
18:40 |
kahrl |
I guess all instances of pathRemoveFile can be replaced by fs::RemoveLastPathComponent |
18:41 |
kahrl |
which handles both '/' and '\\' (the latter only on windows) and is unit tested |
18:43 |
hmmmm |
agreed, also the "abnormal termination" that is commonly mistaken for a "crash" that happens when mainmenu lua fails to initialize is not very user friendly. I recommend that on Windows, at least, it should pop open a modal MessageBox explaining the error |
18:43 |
est31 |
lua is part of the engine |
18:43 |
est31 |
if it has a fatal error, you can't run it anymore |
18:44 |
est31 |
but yes messagebox sounds good |
18:44 |
|
SudoAptGetPlay joined #minetest-dev |
18:45 |
est31 |
also kahrl's idea |
18:46 |
est31 |
btw, why isn't filesys.{cpp,h} inside util? |
18:47 |
est31 |
porting should be there too I think. |
18:48 |
kahrl |
I thought about moving them but at the time there were a number of PRs for them |
18:48 |
kahrl |
not sure how many there are now |
18:48 |
hmmmm |
probably none. none as far as i'm aware of |
18:49 |
hmmmm |
i think we should begin the (slow) process of reorganizing the directory structure |
18:49 |
hmmmm |
right now I'm splitting test.cpp up into individual files for each thing being unit tested and putting them into a 'unittests' directory |
18:50 |
hmmmm |
so basically whenever we are confident there aren't any PRs that modify a certain file, we can move it into its correct place (i.e. client/ or server/) |
18:50 |
kahrl |
sounds like a good plan |
18:51 |
kahrl |
maybe have some kind of mini-roadmap that shows how the directory structure should look like once this process is finished |
18:52 |
hmmmm |
will do |
18:57 |
Krock |
kahrl, I'm not sure if it's the best way to convert the char* to std:string to use "fs::RemoveLastPathComponent()" and then again backwards to not break the other platforms codes. As far as I see, only the Windows part returns a file path. |
19:26 |
|
neoascetic joined #minetest-dev |
19:40 |
|
SudoAptGetPlay joined #minetest-dev |
19:51 |
|
err404 joined #minetest-dev |
19:52 |
|
proller joined #minetest-dev |
20:06 |
OldCoder |
est31, thank you |
20:11 |
ElectronLibre |
To anyone who might be interested game#494 is updated with a better way to store flowers' datas and some little fixes to Ombridride's code. |
20:11 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/494 -- Simplified flowers registration by LeMagnesium |
20:16 |
|
Wayward_Tab joined #minetest-dev |
20:28 |
|
proller joined #minetest-dev |
20:29 |
|
Wayward_Tab joined #minetest-dev |
20:43 |
OldCoder |
est31, unless the server list servers were designed to report to each other. But this is probably for longer-term discussion. |
20:44 |
est31 |
yes |
20:44 |
est31 |
it is possible to make them do that, yes indeed. |
20:47 |
OldCoder |
Thank you |
21:09 |
|
ElectronLibre left #minetest-dev |
21:11 |
|
Hijiri joined #minetest-dev |
21:13 |
|
devmarth joined #minetest-dev |
21:19 |
|
ElectronLibre joined #minetest-dev |
21:32 |
|
luizrpgluiz joined #minetest-dev |
21:33 |
luizrpgluiz |
chatting with colors will be very good at the game |
21:39 |
kahrl |
another unreviewed commit by nerzhul? |
21:40 |
|
OldCoder joined #minetest-dev |
21:46 |
luizrpgluiz |
gave me the github link for me to see what will be the new version of minetest, then I read the news :) |
22:05 |
|
luizrpgluiz left #minetest-dev |
22:06 |
|
AnotherBrick joined #minetest-dev |
22:11 |
hmmmm |
alright that's it |
22:11 |
hmmmm |
i hate to be the one to do this |
22:13 |
hmmmm |
https://github.com/minetest/minetest/commit/e0eec201a18a0741114094b600f765313a838bfb |
22:26 |
VanessaE |
ouch. |
22:42 |
hmmmm |
we tolerated it too much |
22:43 |
VanessaE |
well, I can't say I disagree with you having reverted it. |
22:43 |
VanessaE |
just not used to seeing an outright "all right, byte me" response like that :) |
22:43 |
|
Hijiri joined #minetest-dev |
22:57 |
|
Hijiri joined #minetest-dev |
23:11 |
|
Gethiox joined #minetest-dev |
23:15 |
|
Wayward_Tab joined #minetest-dev |
23:39 |
|
Player_2 joined #minetest-dev |