Time Nick Message 02:19 us`0gb I disabled the fog in the Android game and the graphics weirded out. I'm seeing sky-colored discolorations on every node. 02:20 us`0gb Most of these blemishes are in the form of stripes that fade as they move from the center. 02:21 us`0gb I'd take a screenshot, but I don't knowhow on this thing. 03:22 ShadowNinja us`0gb: If it's a semi-recent device (4.0+ or so) power & volume down should do it. 03:31 us`0gb ShadowNinja: 4.2, no effect. 14:17 rubenwardy What is a "user-agent" in terms of Minetest? 15:34 ShadowNinja celeron55: Is Minetest a registered trademark? 15:37 celeron55 it's not registered, but it is a trademark 16:01 us`0gb We should start spelling it as Minetest™ then. 16:08 rubenwardy ofc 16:09 ShadowNinja Either that's the box character, or my font doesn't support it. 16:10 ShadowNinja sapier: #1226 16:10 ShadowBot https://github.com/minetest/minetest/issues/1226 -- [WIP] Remove dependency on marshal and push the Lua error handler only once by ShadowNinja 16:12 ShadowNinja sapier: Also, what's the rationalle for UpperCamelCase/lowerCamelCase in the async classes? lowerCamelCase or underscore_style is normally used for things that aren't types. 16:12 sapier already looking at it ... haven checked much but you're wrolng about lua -> cpp move cpp is part of scriptapi providing interface to access FROM cpp ... I don't know of any way to use async from core 16:14 sapier In any case this move should be done on a separate commit as it's impossible to review your changes in this files 16:15 sapier why do you add "minetest" to mainmenu? 16:15 ShadowNinja sapier: Lua doesn't call anything in s_async. 16:15 sapier async is a pure lua feature not part of scriptings core interface 16:15 ShadowNinja For serialize.lua. I'm not sure why a seperate engine table was made in the first place. 16:16 ShadowNinja Otherwise all common files need local tbl = minetest or engine or similar. 16:16 sapier because of starting transition from "minetest" name to some generic 16:16 sapier minetest core is supposed to be a engine for arbitrary games why does api have to contain "minetest"? 16:16 ShadowNinja Ah, then change it for the game API too and add minetest = engine to builtin.lua 16:17 sapier I don't change anythin prior checking all of it 16:18 sapier and later it's gonna be your task to fix it 16:18 us`0gb sapier: The engine itself is called Minetest maybe? 16:18 ShadowNinja us`0gb: But there's freeminer. 16:18 sapier STOP 16:18 us`0gb ShadowNinja: Isn't Freeminer a fork of the engine? 16:18 sapier don't argue about names we've got enough tecnical things to check first 16:19 * us`0gb sets +q on 0gb.us@* 16:21 ShadowNinja sapier: So, I agree with the minetest -> engine change, but think it should be done everywhere. But why are you using UpperCamelCase for code and data? 16:22 sapier most likely I did work on code using that style right before writing it ... I hate working on different code I'm allways critizized for using style of oposit code in each one 16:23 ShadowNinja Ah. I'll fix that then, since this is already a pretty big change. 16:24 sapier please first split the file move from your other changes 16:24 sapier noone can check changes in those two files 16:25 sapier did you do any performance check comparing marshal to dump? 16:26 ShadowNinja No, but as long as they both finish within a few miliseconds it doesn't really matter. 16:26 ShadowNinja And can you check if the broken LuaJIT version checking is still relevant? 16:27 sapier first of all I'm gonna check if dump works at all as I decided not to use it because of failing to serialize a full class of functions ... if this didn't improve since that time this fix is broken by definition 16:29 sapier btw how do you wanna fix the issue loadstring not beeing possible on any security relevant lua engine? 16:29 sapier e.g. client side lua 16:30 ShadowNinja sapier: Hmmm... 16:33 ShadowNinja sapier: You'll have to trust builtin and add setfenv(func, engine.restricted_environment) or similat to the caller. 16:34 sapier it's not relevant for async but I won't allow loadstring on client side lua as long as I contribute to minetest 16:37 ShadowNinja sapier: It can't be allowed outside the local builtin install. 16:47 celeron55 sapier: the engine's name is minetest; i don't see what the problem is with the minetest namespace 16:47 celeron55 more so, it's bad practice to have a generic namespace like that 16:48 celeron55 maybe some lua library has something called "engine" in it if they have named things badly 16:48 celeron55 but such definitely won't have anything called "minetest" - that is the point of having a name 16:54 sapier I wonder why this comes into mind about half a year after it was introduced ... I don't care about names as long as I don't have work to change them back and forward so if everyone wants to change it I suggest the one changeing it everywhere 17:01 sapier ok performance of string.dump seems to be even better then marshal (using luajit) ... guess it'd be different on lua but if someone needs performance that badly he's gonna need luajit anyway 17:01 sapier ShadowNinja: we don't "need" the check any longer for our builtin features but luajit still is broken 17:02 sapier btw using serialize for data isn't compatible to current way of doing it you'd need to use dump for them too 17:03 sapier this is relevant if someone uses some sort of "object" containing functions in lua 17:07 sapier s_item.cpp changes are not related to async at all, nor are they in a file changed for it make a different commit ... why do people always believe to mix up things ... thats so proller 17:07 sapier s_inventory.cpp too 17:08 sapier s_entity.cpp too 17:08 ShadowNinja sapier: string.dump dumps only functions. We already have minetest.(de)serialize for data, and an option to serialize functions could be added (if t == "function" then s = s.."loadstring("..serialize_string(string.dump(x))..")" end 17:08 sapier minetest.deserialize doesn't dump functions 17:08 sapier then add it and use it for everything 17:08 ShadowNinja sapier: Yes, I know. Does it need to? 17:09 sapier if you want this to be a 100% replacement and not 90% only ... yes 17:09 ShadowNinja I can try to seperate the changes. And 90% seems close enough, as long as functions aren't being passed already. 17:10 sapier no 90% is crap 17:11 sapier either 100% or nothing ... to often people never add the missing 10% once they're beta code is merged ... and async is meant to be made available for minetest api too 17:12 sapier and I don't have any interest in (re) adding function passing again once I need it for mobf ... the whole api is originaly meant for performance hungry things like mobs ... mainmenu was just a simple (mostly) uncritical testcase 17:13 sapier I guess separating the inventory entity and item things is just commiting them in a second step instead of all at once 17:14 sapier none of our code uses space after if why do you change this all around? 17:16 celeron55 i guess it's the official coding style these days 17:17 sapier I consider this to be code style fetishism ;-) 17:17 celeron55 i will always hate spaces there though 17:18 sapier That's one of the sideeffects defining project codestyle to be "kernel" + something ... if kernel changes in a year we've got to change anything again ;-) 17:19 sapier Another point local variables in for loops. Althogh I hate iterators defined there bloating code (as long as we don't have cx11), I don't think those variables should be declared prior for loop polluting namespace of whole function 17:20 ShadowNinja sapier: Actually a lot of code does. 17:20 sapier C code does have to do so because of ansi standard but that's not required in c++ and as I said IMHO that's bad style 17:21 celeron55 it's definitely bad style in C++ 17:21 sapier variable scope should be as minimal as possible 17:21 ShadowNinja sapier: We stay at the version of the kernel code style that was added at that time unless we explictly chose to use the new style. 17:22 sapier come one ShadowNinja in about a year you/someone're gonna claim "this isn't new but has ever been this way" 17:22 ShadowNinja sapier: Hmmm, good point on the scope. 17:22 sapier but no style discussion again that's not gonna give any benefit 17:23 ShadowNinja Maybe we should have a discussion about code style. But this is one of those thinks where it's unlikely that developers will budge much. 17:23 sapier wow I'm disappointed ... s_server.cpp L84 ... you missed a space 17:23 sapier ;-) 17:23 ShadowNinja sapier: I only changed ones that were nearby my edits. 17:24 sapier for() loop thing isn't code style but coding style 17:24 sapier you changed L84 ;-P 17:24 ShadowNinja (To prevent breaking pulls) 17:24 ShadowNinja Alright, I'll check it. 17:24 sapier you even added a space there but missed a second one ;-) 17:25 ShadowNinja Oh, after. 17:26 sapier MAINMENU_ASYNC_THREADS I'd not remove this to be the number ... or is the define meant to be the threads semselfs? 17:26 sapier -s+th 17:30 ShadowNinja The fact that it's a number seems fairly obvious. That could be changed though. 17:30 sapier scripting_mainmenu.cpp L88++ you're a changeing fetishist aren't you ;-) 17:31 sapier it's only obvious at the define (of course) but not where it's used 17:33 sapier ok so imho three things are left 1) split that pull to reasonable parts, especially the moved files are very very hard to check if done this way (and of course the unrelated changes shouldn't be mixed up) ... 2) add the missing features (primary passing lua tables containing functions themselfs as parameters) 17:33 sapier and 3rd .. the cpp_api lua_api thing ... the difference between those two was meant to be cpp_api things provide interface to be called by core while lua_api is code providing interface to lua 17:34 sapier async is not anything provided to core but to lua only 17:35 ShadowNinja sapier: On the contrary, C++ calls things in s_async, but it does not provide a Lua API, l_mainmenu does that. 17:35 sapier no core doesn't call s_async things but just starts it 17:35 ShadowNinja It's also more similar since it's derived (or should be derived) from ScriptApiBase, not ModApiBase. 17:36 sapier still it's not a core interface 17:36 ShadowNinja sapier: It also calls Step(). 17:36 ShadowNinja It's meant to be used by Lua, but it doesn't provide a lua API, just like s_item and the like. 17:36 sapier what exactly is "it" right now? 17:37 ShadowNinja it = [ls]_async.* 17:37 sapier please define it to be a single thing not everything 17:38 sapier to me cpp_api contains functions to access lua data from core 17:38 ShadowNinja BTW, why do lua_api files have a l_ prefix, and cpp_api files have a s_ prefix? 17:38 sapier while lua_api contains functions provided to lua 17:38 ShadowNinja ^ 17:39 sapier because the folder was called scriptapi before and I didn't change everything 17:39 sapier the change was requested 30 minutes prior merge ... as always 17:39 ShadowNinja Lua API functions are provided by lua_api/l_mainmenu.cpp. The core classes for managing lua_State's for the async API are provided by ccp_api/s_async.cpp. 17:40 ShadowNinja I'll remove the l_ and s_ prefixes in a later commit then. 17:40 sapier no you can't 17:40 sapier you're gonna mess up build 17:41 sapier you need those prefixes because our build system just ignores folders 17:41 sapier at least the way it's configured right now ... you can clean it if you want 17:43 ShadowNinja Commit split. 17:43 ShadowNinja It ignores folders? O_O 17:43 sapier at least for headers ... so if you've got any naming conflict e.g. entity.h you're gonna have a lot of trouble 17:44 sapier of course you can use abosolute paths everywhere .. and make sure cmake uses right one at right location 17:45 sapier guess you're gonna find out how to fix it I didn't wanna spend hours on fixing this 17:48 ShadowNinja I'll do that after my async changes are finished to minimize conflicts. 17:50 ShadowNinja sapier: Is killing all threads without waiting safe? A long proccess can stop the menu from exiting. 17:51 sapier no it isn't 17:51 sfan5 https://github.com/minetest/minetest/pull/1227 17:51 sapier killing a thread might cause arbitrary damage 17:51 sfan5 ^ I am going to merge this soon(tm) 17:52 sfan5 ~title 17:52 ShadowBot sfan5: Fix all warnings reported by clang by sfan5 · Pull Request #1227 · minetest/minetest · GitHub 17:52 sapier :-) you're third one to do thins in half a year sfan5 17:52 sfan5 yeah 17:52 sfan5 but this time I'm not going to let it wait 2 months until it's outdated 17:52 sapier I closed my pull 2 or three days ago becaus of beeing outdated 17:53 celeron55 sfan5: what is this collisionbox change 17:53 celeron55 64 17:53 celeron55 - virtual bool getCollisionBox(aabb3f *toset) = 0; 17:53 celeron55 64 17:53 celeron55 + virtual core::aabbox3d* getCollisionBox() = 0; 17:53 celeron55 and the implementations 17:53 sapier collision box is messing up virtual declarations 17:53 sapier interface implemented by derived class has same name but different signature 17:54 sfan5 it now uses aabb3f* which makes more sense than bool foobar(aabf *pointer) 17:55 celeron55 i think it's very wrong 17:55 sfan5 why? 17:55 celeron55 the interface requires that the caller deallocates the result 17:56 celeron55 return it by value or as std::unqiue_ptr or some other sane way 17:56 sfan5 how would I return "no collisionbox" if returned by value 17:57 celeron55 well, for example the way you're changing it away from 17:57 sfan5 so I should make it bool foobar(aabf *pointer) again? 17:58 sapier no matter how it's done the interface should be used everywhere :-) 17:58 sapier not one derived class using this the other one another 17:58 celeron55 or alternatively require that the object stores the collisionbox by itself so that the caller will not have to deallocate anything but instead has to just keep the object existing as long as it uses the collisionbox 17:58 celeron55 i think some other code does it that way but i am not sure (or it could have been changed already) 18:00 celeron55 if you insist on doing it that way, then it should be called "createCollisionBox" 18:01 celeron55 which implies that it allocates a new object that the caller must manage 18:01 sfan5 *changes it back* 18:02 sapier hmm german easter holidays ... now I know why development speeds up right now :-) 18:02 sfan5 lel 18:02 celeron55 lol 18:02 sfan5 you got it 18:02 sfan5 even when I'm on vacation and only have about 1 hours in the morning and 4 in the evening, I still manage to do stuff 18:03 sapier hope people think about cleaning up stuff for merging needs to be done too 18:04 sapier talking about merge ... any objections for #1130? 18:04 ShadowBot https://github.com/minetest/minetest/issues/1130 -- Jthread: remove locks that arent absolutely required by sapier 18:10 celeron55 that will cause errors on thread error detectors like helgrind 18:10 sapier why? 18:11 celeron55 is it valid code in x86 and ARM? is setting and reading a boolean an atomic operation? 18:11 sapier hmm I didn't check on arm by now true 18:11 celeron55 they check that memory isn't accessed from multiple threads without locking 18:11 celeron55 if you haven't tried helgrind, try it 18:11 celeron55 it's one of the tools in valgrind 18:12 celeron55 (valgrind doesn't work with luajit so that needs to be left out from the build first with -DDISABLE_LUAJIT=1) 18:13 sapier I know that's why I added the locks when I did this (and have been punched for using that much locks) ;-) ... but I don't se any way how this might fail ... even if there was a concurrent read the desired function would still remain working 18:13 celeron55 oh well 18:14 celeron55 maybe you should add comments to both of those functions that it may cause warnings but is safe 18:14 sapier even if a invalid value is read the desired behaviour would only be delayed by short time 18:14 celeron55 so that if someone uses helgrind, they can correctly set up suppressions 18:14 celeron55 and don't need to try to guess whether it's intended or not 18:15 celeron55 it's fine to me after you do that 18:21 sfan5 celeron55: fixed it 18:21 sapier ok I'm gonna check on arm too (as I now have a arm device) and add those comments 18:23 celeron55 sfan5: seems fine 18:23 sfan5 meging in 10 mingw 18:23 sfan5 merging* mins* 18:23 sfan5 I can't english today 18:24 sapier seems to be quite less changes ... I think I had quite some more ;-) hope you didn't miss a lot ;-) 18:25 sapier assert(state < sizeof(statenames)); why did you remove this? 18:26 sapier assert(m_list_size <= SEQNUM_MAX+1); and this onw? 18:27 celeron55 i don't see a reason to remove those either 18:28 sapier if interface is used correct they can't happen ok ... but if not they're valid assert cases 18:28 sfan5 1) enum ClientState seems to be guaranteed to be in range (clang said this) [also sizeof(statenames) is incorrect anyway] 18:28 sapier as long as noone calles the function casting int to enum 18:28 sfan5 2) m_list_size is u16, it would overflow and never go above 65536 18:28 ShadowNinja sapier: http://ix.io/bHx/diff 18:29 sapier SEQNUM_MAX doesn't always have to be defined as 65536 18:29 sfan5 but then I can't remove the warning :-( 18:29 sfan5 should I make m_list_size u32? 18:29 sapier values above 65536 aren't possible due to protocol format 18:30 sfan5 IIRC m_list_size does not come from a network packet 18:30 sapier but it'd be better then removing the check 18:30 sfan5 (not directly) 18:30 sapier no but the SEQNUM_MAX is a network parameter 18:31 sapier can we rename ShadowNinja to RenameNinja? ;-) I'm quite sure he'd be glad about beeing renamed :-) 18:33 sapier can we please once and forever define if class MEMBERS shall be named m_ or not ... imho this is crucial if you wanna use code without full blown ide support ... what do others think? 18:33 ShadowNinja sapier: Nope, Charybdis and -seven don't support /sanick or /svsnick or /onick or any similar commend other than RSFNC, which is services-server-only. 18:33 sapier sad renameNinja would match quite good ;-) 18:34 ShadowNinja Use of Hungarian notation is very limited, to g_ for globals (rare in the first place) and m_ for members. The latter is discouraged for newer code (since one can simply notice there is no variable by that name declared in that method's scope), but needed when adding a member to older code for consistency. 18:34 sfan5 sapier: you did not read the code style guidelines 18:34 sfan5 it says members should only be m_ if writing additional code for classes that already use this 18:34 sapier it's discouraged because of you added it and this is one of the major faults in our coding guidelines 18:35 sfan5 >because of you added it 18:35 sfan5 what? 18:35 sapier it's adding quite a lot of code readability while spending ony 2 characters 18:35 ShadowNinja sapier: How did this work on Linux? https://github.com/minetest/minetest/commit/960d731587f58036bd4957f4a77db41e145c2d04 18:35 sapier sorry meant ShadowNinja ;-) 18:35 sfan5 ShadowNinja: it worked with #ifdef _WIN32 18:36 sapier old version never did work 18:36 sapier IMHO removing m_ g_ and things like that are plain wrong and reason of avoidable misstakes 18:37 sapier you always have to check twice if a variable is a member or a local one ... you see it at once if it's prefixed with m: 18:37 ShadowNinja sfan5: Ah. 18:38 ShadowNinja sfan5: Why are the extra () required around core::stringc(x)? 18:38 sfan5 ShadowNinja: ask the clang devs 18:38 sapier g_ shouldn't be required to remove as g_ are always evil 18:39 ShadowNinja Global variables are bad, so a g_ prefix seems fine. 18:39 sapier http://ix.io/bHx/diff IMHO is mostly bad and wrong (it's almost only removing m_) 18:40 sapier why do you add a deque ? vector is already used there what's wrong with it? 18:41 sapier can you please stop changeing things for sake of changeing? 18:41 ShadowNinja sapier: Because it's a queue. 18:41 sapier AND? 18:42 ShadowNinja queue uses a linked list I imagine, making pushing and poping constant-time. 18:43 ShadowNinja Complexity is defined as constant. 18:43 sapier if there was int_liter and int_kg and int_apples we'd use those for counting apples and int_kg for kilogramms because int wouldn't fit any longer? 18:43 ShadowNinja What? 18:44 sapier ok lets first check but for what I remember pushing and popping vector is same complexity 18:44 ShadowNinja Nope, it's O(n). 18:46 ShadowNinja Pops are O(n), pushes are O(1) if no reallocation is needed. 18:46 ShadowNinja deque is O(1) for pops and pushes. 18:46 celeron55 yes those pop_fronts are slow there 18:47 sapier that's wrong 18:47 celeron55 altough, there are probably only like 1 task so they're not 18:47 sapier dequeue is only faster for random inserts 18:47 sapier and for push front 18:48 celeron55 "they provide a functionality similar to vectors, but with efficient insertion and deletion of elements also at the beginning of the sequence, and not only at its end. But, unlike vectors, deques are not guaranteed to store all its elements in contiguous storage locations: accessing elements in a deque by offsetting a pointer to another element causes undefined behavior." 18:48 celeron55 http://www.cplusplus.com/reference/deque/deque/ 18:48 celeron55 i.e. it's approximately the right thing for the job, unlike vector 18:49 sapier but we don't push to front or insert 18:49 celeron55 a vector will always keep the first element at [0] 18:50 celeron55 hmm 18:50 ShadowNinja deque is approximately equivalent to std::list. 18:50 celeron55 i wonder if it's optimized for pop_front to not immediately reallocate, actually 18:50 celeron55 vector i mean 18:50 sfan5 I did some more changed 18:50 celeron55 it could, but it would waste memory 18:50 sfan5 can I merge #1227 now? 18:50 ShadowBot https://github.com/minetest/minetest/issues/1227 -- Fix all warnings reported by clang by sfan5 18:51 sapier http://baptiste-wicht.com/posts/2012/12/cpp-benchmark-vector-list-deque.html 18:52 celeron55 there's no pop_front there 18:52 celeron55 also we can't know what every standard library implementation does; for example g++'s std::list's size() is O(n) while MSVC's is O(1) 18:54 sapier sadly none of the benchmarks tells about pop front ... and in push back vector is slightly faster 18:54 celeron55 the standard clearly says that deque does both pop_front and push_back fast, while it doesn't say that vector would do pop_front fast 18:54 celeron55 to me it seems likely that vector will copy all elements every pop_front 18:57 sapier did anyone benchmark it? ... I've seen to often "assumptions" beeing wrong because of compilers already optimizing way better 18:57 celeron55 how many jobs do you expect there to be at maximum? 18:58 celeron55 if the answer is under 20, the choice of container doesn't matter at all 18:58 ShadowNinja It probably doesn't matter much performance-wise, but a deque is the right container to use. 18:58 sapier if it's no performance result why don't we stop discussing and just keep it the way it's already in? 18:58 celeron55 because correct is better 8) 18:58 sapier it's not correct it's just using something different 18:59 ShadowNinja We use a queue for a queue, rather than an array. 18:59 sapier and no, on reading modstore count gets way beyond 20 19:00 sapier a vector ain't an array 19:00 ShadowNinja A vector is a dynamically realocated array. 19:00 sapier for gods sake change it but it's gonna be you looking for issues in that code 19:01 sapier and I'm gonna stress it to the end once using it for mobf believe me 19:02 ShadowNinja http://pastebin.ubuntu.com/7256975/ O_O 19:04 sapier your problem ;-P 19:06 sapier ShadowNinja: did you do java programming before? 19:07 ShadowNinja Hmmm, It might be because it's empty... 19:07 sapier it can't be empty 19:08 sapier m_JobQueueCounter.Post(); one token per job, there can't be more tokens then jobs queued 19:08 sfan5 something something #1227 something something 19:08 ShadowBot https://github.com/minetest/minetest/issues/1227 -- Fix all warnings reported by clang by sfan5 19:08 ShadowNinja Oh, derp. I was poping from the wrong queue. 19:09 sapier that's happening if you change working code ;-) 19:09 sapier especially if you change lots of things same time 19:10 ShadowNinja Well, it works now. 19:10 ShadowNinja http://ix.io/bHA/diff 19:11 sapier no need to check withing that much changes I don't see the relevant things any longer maybe others are capable of doing it I'm not 19:11 sapier well to be more exact I don't wanna spend hours for looking for s <-> S 19:12 sapier finding out which variable is global member or local and things like that 19:12 sapier especially not in diffs ... where it's impossible to decide about a variable 19:13 sapier who the hell did discourage m_? proller? 19:13 sapier xyz? 19:13 sapier celeron? 19:13 ShadowNinja sfan5: That seems fine if you fix (or have fixed) the other mbstowcs return value ignore and fix the commit message. 19:13 sfan5 ShadowNinja: gcc still complains about the return value 19:14 sfan5 I don't know how to make it shut up 19:14 ShadowNinja sapier: hmmm. 19:14 ShadowNinja sfan5: I mean, there was annother one that has to be ignored too. 19:14 sapier can you remove mbtowc completely in there? that's a function beeing very os specific and shouldn't be used outside of porting 19:14 proller m_ is stupidest thing 19:15 sapier of course proller you're able to remember thousands of variables on using kwrite to edit code 19:16 sfan5 sapier: it is in an #ifdef linux 19:16 sapier that's even worse 19:16 sapier ok change it another time 19:16 sfan5 ShadowNinja: https://github.com/sfan5/minetest/commit/7614d0cab23ab67a7b72b7fb3e02a37d0ee82b76#diff-420ef4c204f7735e35d574866057a20bR560 19:16 sfan5 gcc still complains on the same line 19:18 sapier ok seems I'm the only one who want's code to be easyly readable so I wont fix any bugs in code that was changed from readable to non readable ... 19:19 ShadowNinja sfan5: There is a second mbtowc with an ignored return value. 19:19 sfan5 where? 19:21 ShadowNinja intlGUIEditBox.cpp 19:23 sfan5 line? 19:23 ShadowNinja sfan5: And this is related: https://github.com/minetest/minetest/issues/1158 19:23 sfan5 I tried to fix that 19:23 sfan5 but gcc still complains 19:29 ShadowNinja Can #808 be closed? 19:29 ShadowBot https://github.com/minetest/minetest/issues/808 -- Static Analysis Warnings 19:30 ShadowNinja sfan5: grep mbtowcs src/intlGUIEditBox.cpp 19:30 sfan5 probably 19:30 ShadowNinja Around 276. 19:31 ShadowNinja You can probably add a -Wno-X if casting to void doesn't fix it. 19:32 sfan5 no, those warnings can be useful 19:32 sfan5 let's just leave them 19:32 sfan5 gcc sucks anyway 19:32 sfan5 clang does not report any warnings 19:35 sfan5 ShadowNinja: care to agree so I can push it? 19:39 sapier have those warnings been there before? 19:40 sfan5 dunno 19:40 sapier if they have been there ignore them if they havent fix them ... it's quite simple 19:40 sfan5 well 19:40 sfan5 I did definitely not add them 19:41 sapier it's useless to fix warnings by adding new ones so if your changes would be source of warning that'd be a problem if not it's obviously not your problem 19:42 sapier but by now gcc is still mainstream compiler and clang will take some time to get mainstream (if ever) 19:42 sfan5 gcc only gives 3 warnings (client: 2, server: 1) when compiling 19:42 sfan5 (atleast on travis-ci) 19:42 sapier how did we configure gcc? 19:43 sapier no doubt clang is way mor strict ... but you can get a lot of warnings from gcc too 19:57 ShadowNinja sfan5: https://github.com/sfan5/minetest/commit/d6f193434b7c31d4b9771a0cbcc1c0e59cf125d3#diff-a8c2ccea72474c4e178b267a03b8610eR212 Why? Default is 0, right? 19:58 sfan5 I need the value for 'invalid' to be > 0 because it's signed 19:58 sfan5 unsigned* 19:59 ShadowNinja OK. You could also add DECO_INVAILID somewhere in there and use that. 20:00 sfan5 >could 20:00 ShadowNinja Otherwise it seems good (with squashing). 20:00 sfan5 thank you! 20:00 sfan5 \o/ 20:00 sfan5 pushing! 20:00 sfan5 finally we have this sorted out 20:01 ShadowNinja sfan5: Now can you check this? :-) https://github.com/minetest/minetest/pull/1226 20:02 sfan5 I'll try, I'm not that good with lua stuff 20:02 ShadowNinja sfan5: And this, mich simpler: http://sprunge.us/NRgQ 20:02 ShadowNinja +?diff 20:03 sfan5 why are you adding {'s 20:03 ShadowNinja You mean if (x) y(); -> if (x) { y(); } ? 20:03 sfan5 yes 20:04 ShadowNinja It reduces confusion and prevents errors when adding something new to the block. Eg, if (x) y(); z(); Won't work as if (x) { y(); z(); } 20:05 sfan5 it increases confusion 20:05 sfan5 rest seems fine 20:07 ShadowNinja sfan5: And the DB tweak? 20:07 sfan5 I meant that one with rest seems fine 20:07 ShadowNinja OK. 20:13 sapier return (u64) pos.Z * 0x1000000 come on guys can you pleas use shift where you do shift? 20:13 sapier how much other things like that are burried in there? 20:14 ShadowNinja sapier: I had some issues with a bit shift there. 20:14 sapier what issues? 20:16 ShadowNinja sapier: Try it and load a map, then look around at 0, 10, 0. :-) 20:16 ShadowNinja The map itself shouldn't be corrupted as long as you don't touch any nodes. 20:17 sapier that doesn't make sense ShadowNinja 20:18 sapier shift and multiplication by pot2 values are identical operations ... despite of insane compiler may really use multiplication in later case 20:18 sapier none I know does 20:18 ShadowNinja sapier: The map is loaded into memory incorrectly, but it won't be written unless you touch something. 20:18 ShadowNinja sapier: Negative numbers cause issues. 20:18 sapier did you debug it? 20:18 sfan5 how about using abs() ? 20:19 sapier there aren't negative numbers for u64 20:19 sapier or is map format broken as of beginning and we abuse a overflow to keep compatibility? 20:19 * ShadowNinja tries again. 20:20 ShadowNinja sapier: There are apperently negative indices, I don't know how, considering that we don't use the last bit. 20:20 sapier the whole function doesn't make sense it's casted to unsigned for calculation and returned as signed 20:24 ShadowNinja I got it to work. I had order-of-operations wrong. I don't know what is happening with the signedness. 20:25 ShadowNinja http://ix.io/bHG/diff Pushing in a minute 20:25 sapier wait a second 20:26 sapier 0xfffffffffc000001 is composed of z=0xfffc y=0x0000 and x=0x001 20:26 ShadowNinja Yes. 20:26 ShadowNinja Er, ffc rather. 20:27 sapier I'm missing some bytes 20:27 sapier if there was a 16 bit shift ... I'd have expected something like 0xFFFFFFFc00000001 20:27 ShadowNinja Only part of the 64-bit number is used. 20:28 ShadowNinja There's only a 12 bit shift because it's the BLOCK position, not the NODE position. 20:28 sapier ok in this case it's gonna make more sense 20:28 sapier and that's reason why I prefere << number for * somevalue 20:29 ShadowNinja So, seems fine? 20:30 sapier I guess you tested it, I don't see a obvious error 20:31 sapier I don't even wanna think about how much it's gonna be to rebase android port :-( 21:09 PilzAdam sfan5, https://github.com/minetest/minetest/issues/1158#issuecomment-40527349 21:22 zat Guys please consider making the names of the players case-insensitive while still displaying in the case the user originally registered with. 23:51 ShadowNinja sfan5: Some changes to the mapper: http://ix.io/bHU/diff Performance difference: http://pastebin.ubuntu.com/7258368/ 23:52 ShadowNinja This makes both DBs faster, and SQLite3 slower. The reason is because each block is fetched individually rather than a whole Z-slice at a time.