Minetest logo

IRC log for #minetest-dev, 2015-07-16

| Channels | #minetest-dev index | Today | | Google Search | Plaintext

All times shown according to UTC.

Time Nick Message
00:51 stormchaser3000 joined #minetest-dev
01:19 blaise joined #minetest-dev
01:20 kahrl_ joined #minetest-dev
01:37 hmmmm joined #minetest-dev
01:47 blaise joined #minetest-dev
01:55 asl97 joined #minetest-dev
02:53 werwerwer joined #minetest-dev
03:06 Etzos joined #minetest-dev
04:38 stormchaser3000 joined #minetest-dev
05:20 leat1 joined #minetest-dev
05:41 leat1 joined #minetest-dev
05:46 Hunterz joined #minetest-dev
05:52 blaise joined #minetest-dev
05:59 Zeno` joined #minetest-dev
06:00 Zeno` est31, I updated my comment (just in case you missed it) ;)
06:00 Zeno` just checking the std now, but I'm 99.999999% positive bitshifts on signed numbers are implementation-defined
06:00 nrz Hey Zeno :D
06:01 Zeno` hi. been a while :D
06:01 nrz of course :)
06:01 Zeno` hehe, ok, left shifts are undefined behaviour
06:02 nrzkt joined #minetest-dev
06:02 nrzkt better like this :)
06:02 Zeno` :P
06:02 Zeno` how's things?
06:03 est31 hey Zeno`
06:03 est31 so what do you think should be done about it
06:03 est31 also, do you like the feature at all
06:03 est31 paramat didnt
06:04 chchjesus_ joined #minetest-dev
06:05 Zeno` est31, I guess I'd cast them to u64 before shifting
06:05 Zeno` it's just to get a unique number, right?
06:05 Zeno` number/seed
06:05 est31 yes
06:06 Zeno` yeah, I doubt the undefined behaviour matters in that case but I'd cast anyway
06:06 Zeno` (for the sake of "properness" heh)
06:06 Zeno` I don't mind the feature btw
06:09 Zeno` I've also changed my mind. Bitshifts are the clearest way to do what you're doing
06:09 nrzkt u64 should be used for every unique ID
06:11 hmmmm undefined?
06:12 Zeno` hmmmm, yes
06:12 hmmmm I thought the operation forces them to implicitly convert to an unsigned type
06:12 Zeno` apparently not
06:12 Zeno` 6.5.7
06:12 Zeno` para 4
06:12 hmmmm is this from the C++ spec or C spec
06:13 hmmmm i've been noticing subtle differences between the two in regard to type promotion and conversion
06:14 Zeno` C
06:14 Zeno` umm, I don't think there's a difference there... but
06:14 Zeno` looking
06:15 Zeno` the C++ standard says that if anything isn't mentioned than the C standard applies
06:15 Zeno` still looking...
06:15 est31 hmmmm, what do you think about the feature
06:16 hmmmm what feature
06:16 Zeno` yep undefined
06:16 Zeno` 5.8.2
06:17 hmmmm heh good to know
06:17 est31 the one i add in #2910
06:17 ShadowBot https://github.com/minetest/minetest/issues/2910 -- Treegen: Add enable_unique_ids option by est31
06:17 hmmmm although I don't think I ever do bit operations on signed types to begin with
06:17 hmmmm out of principle
06:17 Zeno` it's only undefined if the value is actually negative though
06:18 hmmmm aren't enum values implicitly signed?  i.e. enum values defined with (1 << n) are relying on undefined behavior
06:18 Zeno` Otherwise, if
06:18 Zeno` E1
06:18 Zeno` has a signed type and non-negative value, and
06:18 Zeno` E1
06:18 Zeno` ×
06:18 Zeno` 2
06:18 Zeno` E2
06:18 Zeno` is representable
06:18 Zeno` in the corresponding unsigned type of the result type, then that value, converted to the result type, is the
06:18 hmmmm ahh okay
06:18 Zeno` resulting value; otherwise, the behavior is undefined
06:18 Zeno` shit
06:18 Zeno` didn't know it'd do that
06:18 hmmmm fwiw I absolutely can't stand it when people misuse enum
06:19 hmmmm using enum as set of flags
06:19 hmmmm using enum to define a constant integer value
06:21 Zeno` I use 1U << :)
06:21 Zeno` but probably not in an enum
06:21 Zeno` #define usually
06:22 est31 isn't enum meant to supersede #define ??
06:22 hmmmm absolutely not
06:22 est31 i mean for that usecase
06:22 hmmmm not for flag-types, no.
06:25 est31 I have mixed enums and flags a *bit* for srp
06:25 est31 the server sends a list of supported auth protocols as a flagbyte
06:25 est31 then the client decodes it, and choses one auth mechanism
06:26 est31 that is then stored as enum
06:26 hmmmm that is fine
06:26 hmmmm selecting one option of numerous
06:26 hmmmm storing numerous options... is not fine
06:27 hmmmm an enum is supposed to be a data type where only those enum options are the possible values
06:27 hmmmm it may be undefined behavior to attempt to store a value not part of an enum into an enum
06:27 Zeno` pretty sure it *is* undefined
06:27 hmmmm but at the very least, you lose the advantage in debugging
06:29 kahrl_ the C++ standard defines enumeration types in terms of their underlying integral type
06:29 kahrl_ so I'm pretty sure using them as flags is legal
06:30 Zeno` but, hmmmm, you could have enum { BLAH = 1U << 0; FOO = 1U << 1; BLAH_AND_FOO = 1U << 0 & 1U << 1; } I suppose
06:30 hmmmm the underlying integral type's only assurance is that it's convertable to int, no?
06:30 nore joined #minetest-dev
06:30 Zeno` I think what hmmmm is saying is that you should store the byte flags (combined) in the enum
06:30 hmmmm which means you don't have any hard guarantees on how many options you can fit beyond 16
06:30 hmmmm or rather, 15 :)
06:30 Zeno` which I'd agree with
06:31 Zeno` hmm
06:31 Zeno` I'm confused by what's being said now, lol
06:31 hmmmm Zeno`: yes, you can have that.  but specifying each and every combination defeats the purpose of flags
06:31 Zeno` hmmmm, should NOT* store the byte flags (combined)
06:31 hmmmm right
06:32 Zeno` yeah, that would seem like abuse of the enum
06:33 hmmmm hmm
06:33 Zeno` I've never seen code that does that though I don't think
06:33 hmmmm i can't find my C++11 spec
06:33 hmmmm I have...
06:33 hmmmm it's horrifying because the people who do that believe they are so clever
06:33 hmmmm "look at all these high level C++ constructs I am using!"
06:33 chchjesus_ left #minetest-dev
06:34 kahrl_ hmmmm: http://paste.debian.net/282818/
06:34 kahrl_ see 6. specifically
06:34 hmmmm nice
06:36 RealBadAngel joined #minetest-dev
06:37 Zeno` kahrl_, you don't agree that it would be bad form to write something like....  MyEnumType flags = ENUMA | ENUMB;
06:37 Zeno` I don't like that much at all. heh
06:38 kahrl_ does that even work? wouldn't you have to define an operator| first?
06:38 Zeno` no sure
06:38 Zeno` not*
06:38 kahrl_ like in http://stackoverflow.com/a/1448478
06:39 Zeno` http://codepad.org/Vp3YVtdF
06:39 Zeno` heh
06:42 kahrl_ ugh, how do I tell github markdown to not change tabs to spaces in a code block
06:42 hmmmm yuck
06:43 kahrl_ I guess I'll have to upload my tiny patch as a gist or commit
06:43 hmmmm nobody has tried this so far but would you agree to banning enum-flags?
06:43 Zeno` it works fine now that I've cast the expression
06:43 Zeno` kind of ugly though lol
06:44 Zeno` fully conforming and elegant: http://codepad.org/tjzhmeys
06:45 hmmmm hm i'm surprised you don't need to cast A and B to int first
06:46 Zeno` http://codepad.org/na6MkM2E
06:46 Zeno` I am guessing you can't actually use bitwise operators on enum values
06:46 Zeno` ?
06:46 Zeno` I never knew that. Probably because it never occurred to me to try it
06:47 hmmmm no
06:47 hmmmm you can
06:47 hmmmm look you're doing it with A and B the line above
06:47 Zeno` you can, but not assign them back to the enum
06:47 hmmmm the error is converting it back to blah
06:47 Zeno` yeah
06:47 Zeno` so you'd have to jump through hoops to do the ugliness anyway
06:47 hmmmm right.
06:47 hmmmm so anyway
06:48 hmmmm if you do this, it might be valid C++, but you lose all advantages of enums in debugging
06:48 Zeno` interesting diversion for 30 minutes :)
06:48 hmmmm and then there's the uglyness
06:48 hmmmm the only people who would write code like this are those holier-than-thou language "experts"
06:48 hmmmm you know the ones i mean
06:50 Zeno` haha :)
06:50 Zeno` Linus
06:51 Zeno` lol, just kidding
06:51 Zeno` I had a program once.. an expression parser thing
06:51 Zeno` wonder what I did with it
06:51 leat1 joined #minetest-dev
06:52 Zeno` kahrl_, you wrote it?
06:52 Zeno` yikes! Time! bbl
06:52 Zeno` cyas on the flipside
06:57 Player_2 joined #minetest-dev
06:58 blaise joined #minetest-dev
06:59 RealBadAngel joined #minetest-dev
07:04 Krock joined #minetest-dev
07:41 est31 opinions on #2916
07:42 ShadowBot https://github.com/minetest/minetest/issues/2916 -- Use Z-order curves? Or split up the coordinates?
07:43 crazyR joined #minetest-dev
07:43 crazyR joined #minetest-dev
07:45 blaise joined #minetest-dev
07:47 kilbith joined #minetest-dev
07:52 kilbith left #minetest-dev
07:54 crecca joined #minetest-dev
08:00 Yepoleb_ joined #minetest-dev
08:12 leat1 joined #minetest-dev
08:52 Darcidride joined #minetest-dev
08:53 blaise joined #minetest-dev
09:13 Calinou joined #minetest-dev
09:33 Amaz joined #minetest-dev
09:36 WSDguy2014 joined #minetest-dev
09:37 WSDguy2014 left #minetest-dev
09:41 Taoki joined #minetest-dev
09:46 jin_xi joined #minetest-dev
09:48 Fritigern joined #minetest-dev
09:51 VanessaE another backtrace added to #2913
09:51 ShadowBot https://github.com/minetest/minetest/issues/2913 -- Unexplained, random crashes (segfaults, aborts, OOM)
09:52 est31 VanessaE, fix the formating
09:52 VanessaE oops
09:52 est31 well this line the lines are broken at least
09:52 VanessaE fixed.
09:52 VanessaE forgot to wrap the paste in ``` ```
09:53 WSDguy2014 joined #minetest-dev
09:54 WSDguy2014 left #minetest-dev
09:55 est31 first it crashed at the deallocation of a SharedBuffer, now it cries at the allocation of a SharedBuffer.
09:55 est31 the first one was an assertion failure
09:55 est31 whats this
09:55 WSDguy2014 joined #minetest-dev
09:55 WSDguy2014 left #minetest-dev
09:57 VanessaE beats me, this last one didn't even allow the engine to throw a "shutting down..." message to clients
10:06 nrzkt you should find which SharedBuffer is used and why it's misused
10:06 est31 nrzkt, you are the network guy
10:07 nrzkt SharedBuffer is not used in network
10:07 est31 #6 0x00000000005fbaa6 in con::ConnectionReceiveThread::receive (this=0x7fffffffe438) at /home/minetest/minetest_core/s​rc/network/connection.cpp:2099
10:08 nrzkt i miss this line, sorry.
10:08 nrzkt I'm at work i cannot look at this atm
10:09 nrzkt and kicking player on shutdown is not a role of a mod. Mod is an extension for the core. Core doesn't need to be extended for kicking player at shutdown, it's a core normal function.
10:09 est31 nrzkt, if we dont kick players on a crash the feature is half assed and we don't meet user's expectations
10:10 nrzkt the crash issue should be avoided by using a ping packet function
10:10 est31 mods can do this stuff already, and no need for c++ on this
10:10 est31 if there werent the crash issue, i would even have agreed it being in lua
10:10 nrzkt proper shutdown should be handled by core, kicking all player before unallocating everything
10:11 est31 yes, thats ok, but only if together when we look for a crash
10:12 est31 also kicking should only be done for players using the legacy protocol
10:12 nrzkt why ?
10:12 est31 because ERROR: ACCESS DENIED is bad
10:13 est31 or ERROR:ACCESS DENIED. REASON: KICKED: REASON: SERVER SHUTTING DOWN
10:13 est31 instead it should read SERVER SHUTTING DOWN
10:13 nrzkt right
10:14 est31 so we need a proper "TOCLIENT_SERVER_SHUTDOWN" packet
10:14 est31 or however its named
10:15 nrzkt agree with this.
10:15 nrzkt but what does kick player function ?
10:15 nrzkt ACCESS DENID ?
10:15 nrzkt because it remove the datas and then the UDP socket doesn't work ?
10:15 est31 yes, it shows that in the client last time i checked
10:16 nrzkt okay i see
10:16 nrzkt for legacy we don't have choice
10:16 est31 if i see a way to do it, I will change it to "you have been kicked", ok?
10:16 TBC_x better would be Disconnected fom server\nKicked: $REASON and Disconnected from server\nServer shutdown
10:17 TBC_x rather than adding specialized packet IMHO
10:17 nrzkt i think the TOCLIENT_SERVER_SHUTDOWN is a good idea, and this packet show the connection error screen and close properly resources clientside
10:17 nrzkt or better
10:17 nrzkt TOCLIENT_KICK
10:17 TBC_x so string like Disconnected from server\n$CUSTOM_STRING
10:18 nrzkt with a u8 REASON std::string custom_string
10:18 est31 ah i see
10:18 nrzkt like in the new error message at connection,
10:18 est31 TBC_x, remember that the message is the same for wrong password and kicks
10:18 nrzkt u8 reason => classic messages, which can be translated
10:19 FR^2 joined #minetest-dev
10:19 nrzkt and reason == KICKREASON_CUSTOM we read string
10:20 nrzkt then, mods can kick with a custom reason :)
10:21 TBC_x that makes sense
10:23 nrzkt okay then
10:24 nrzkt est31, are you hot to code this, or minetest will wait for my PR in next days ? :)
10:28 est31 nrzkt, take your time, and make it properly
10:32 nrzkt i propose first, to add the function to kick players with the current method at shutdown
10:32 nrzkt and next, the new function
10:33 VanessaE wtf guys?
10:33 VanessaE are you insane?
10:33 VanessaE telling the player they've been KICKED?
10:33 VanessaE no
10:33 VanessaE that's a flat out NO
10:33 VanessaE players think kick -- ban
10:33 VanessaE ==
10:34 nrzkt if you ban, tell them you ban
10:34 nrzkt :p
10:34 VanessaE it should show one and ONLY one thing to the client:  "Disconnected -- server shutting down... or similar.
10:34 VanessaE nrzkt: it doesn't work that way in a production server.
10:34 VanessaE ANY kick, to a user, looks like a ban
10:34 VanessaE even if you expressly tell them it's not a ban
10:35 est31 VanessaE, you should comment on #2845
10:35 ShadowBot https://github.com/minetest/minetest/issues/2845 -- Kick players on server shutdown by red-001
10:35 nrzkt VanessaE: we are talking technically there.
10:36 nrzkt the message is a futile string, KICK is a technical word there
10:36 nrzkt Server Shutdown is the message shown to client.
10:36 VanessaE nrzkt: yes and when you send a kick command to the client, it's gonna tell the user they've been kicked.
10:36 nrzkt KICK is generic for our packet
10:37 nrzkt we will not create a packet to kick and a packet to shutdown server
10:38 nrzkt it's the same way to handle it on client
10:38 TBC_x Wouldn't it be better to replace bools with enums in void Client::addUpdateMeshTaskWithEdge(v3s16 blockpos, bool ack_to_server, bool urgent) and such?
10:39 est31 ??
10:39 VanessaE est31: commented.
10:39 TBC_x because addUpdateMeshTaskWithEdge(i->first, false, true) won't tell much
10:40 est31 so you want to do what instead?
10:40 nrzkt thanks VanessaE
10:40 TBC_x addUpdateMeshTaskWithEdge(i->first, NO_ACK, URGENT) for example
10:40 est31 meh, rather no
10:40 TBC_x you know what it means without looking up the definition
10:40 est31 yea has a charme
10:41 est31 but you have to define the enum somewhere
10:41 est31 you will have to think of a nice name for the enum
10:41 est31 then how do the checking?
10:41 est31 If urgency_level == URGENT then
10:41 est31 thats bad
10:41 est31 the user has to look up the enum instead
10:42 TBC_x good point
10:42 est31 what if there is SUPER_URGENT, and URGENT is the "less urgent" one?
10:44 VanessaE sorry to be a hard-ass with my comments here, but I have to deal with total idiot users on an continuous basis, so whatever message is sent to the client, it must be user-friendly above all else.
10:45 TBC_x hmm... namespace Bool { enum Urgent } could solve that
10:46 TBC_x or just use const bool URGENT = true
10:46 est31 ah dude
10:47 TBC_x hehe
10:51 blaise joined #minetest-dev
11:00 est31 joined #minetest-dev
11:01 proller joined #minetest-dev
11:04 est31 rebased #2885, nrzkt would be perhaps good to have network maintainer have a look at it
11:04 ShadowBot https://github.com/minetest/minetest/issues/2885 -- Utf8 based chat by est31
11:04 proller joined #minetest-dev
11:12 FR^2 left #minetest-dev
11:12 stormchaser3000_ joined #minetest-dev
11:32 twoelk joined #minetest-dev
11:42 proller joined #minetest-dev
11:48 Hunterz joined #minetest-dev
11:54 OldCoder joined #minetest-dev
12:08 Darcidride_ joined #minetest-dev
12:11 Darcidride joined #minetest-dev
12:20 Sokomine_ joined #minetest-dev
13:22 proller joined #minetest-dev
13:24 leat2 joined #minetest-dev
13:35 leat2 joined #minetest-dev
13:35 H-H-H joined #minetest-dev
14:01 rubenwardy joined #minetest-dev
14:08 amadin1 joined #minetest-dev
14:23 Darcidride joined #minetest-dev
14:23 Megaf joined #minetest-dev
14:59 kilbith joined #minetest-dev
14:59 kilbith left #minetest-dev
15:12 proller joined #minetest-dev
15:18 hmmmm joined #minetest-dev
15:19 rubenwardy joined #minetest-dev
15:26 Hunterz joined #minetest-dev
15:28 stormchaser3000_ joined #minetest-dev
15:33 blaise joined #minetest-dev
15:40 FR^2 joined #minetest-dev
15:42 nrzkt VanessaE crazyR: https://github.com/minetest/minetest/pull/2918
15:42 nrzkt i push it in 3 hours if no objection
15:43 nrzkt #2918
15:43 ShadowBot https://github.com/minetest/minetest/issues/2918 -- Kick players when shutting down server by nerzhul
15:45 crazyR nrzkt looks good, althought i think build checks have failed. you may wanna have a quick look :)
15:47 nrzkt i will look at home, a++
16:08 nrzkt joined #minetest-dev
16:16 Krock I ran into a struct array problem: http://pastebin.com/DUe6iuMJ What's wrong here? It gives me an error about a misplaced '{' in the line "a_env[ENV_UNKNOWN] = {"
16:16 H-H-H that failed due to not being able to download irlict source
16:18 blaise joined #minetest-dev
16:18 nrzkt #2918 refactored to have a custom string when kicking all players. Also use it when Lua stack exception shutdown occurs
16:18 ShadowBot https://github.com/minetest/minetest/issues/2918 -- Kick players when shutting down server by nerzhul
16:19 nrzkt for core segfault is not handlable with this, because it's a coredump
16:20 Krock oh well, could someone tell me how I could solve that problem?
16:20 nrzkt H-H-H right, i have this error
16:21 nrzkt the next build works properly, hopefully
16:21 H-H-H nrzkt you also do android builds dont you
16:21 nrzkt ofc
16:22 nrzkt it's a Debian VM under BHyve on FreeBSD on my own server
16:22 H-H-H have you tested any recent android builds on hardware /
16:22 nrzkt which is a jenkins slave
16:22 nrzkt no, i haven't do android build since a long time. Is there any problem ?
16:23 H-H-H i can build head for android but theres something wrong with the sound it seems it is seriously loud and distorted however i can build stable 0.4 for android and the sound is fine
16:23 nrzkt i also noticed with clang that SRP needs a little fix, i will talk to est31 to fix those warns
16:23 H-H-H tested on galaxy fame and galaxy note 10.2
16:23 H-H-H 10.1*
16:23 nrzkt i didn't worked on sound, but i could test it maybe
16:23 nrzkt https://jenkins.unix-experience.fr/job/minete​st-android-OfficialRepository-RollingRelease/
16:24 nrzkt have you tested this APK ?
16:24 proller joined #minetest-dev
16:24 H-H-H i used exactly same setup as in ndk and sdk were same when i built the 2 and as i say stable-4.0 works fine just not head
16:24 nrzkt this a daily build which generate an apk for users
16:24 H-H-H i will d/l and test now
16:25 nrzkt you can test many APK, there is a build history, one build per day :)
16:25 H-H-H will have to sign it first :)
16:25 nrzkt oh, you must be logged to dl ?
16:25 H-H-H nope im not logged in
16:25 nrzkt ah oh no, sign the apk
16:25 nrzkt it's not needed, it's a debug apk
16:26 H-H-H i just saw a link that was the unsigned one
16:26 H-H-H it says its minetest-release-unsigned
16:26 nrzkt yes, signing is done on my PC if i remember, i will automate it on jenkins maybe at next release
16:26 H-H-H i can sign it no big deal
16:26 nrzkt it's not needed for debug builds :p
16:27 nrzkt oh no
16:27 nrzkt it's a release build, not a debug build, i looked at the command line
16:27 nrzkt you can test and bissect, this will show a commit interval
16:29 H-H-H signed now installing
16:30 nrzkt good luck for your tests
16:32 H-H-H thanks for your help it seems i must be missing something in my build setup as minetest 0.4.12-315b works fine :|
16:35 H-H-H hmm ok thats not head of minetest thats head of stable -0.4
16:35 jin_xi joined #minetest-dev
16:35 H-H-H i mean master has sound issues
16:36 nrzkt i don't think this is maintained...
16:41 H-H-H yeah it just gets built from the stable-0.4 branch
16:41 H-H-H try building an apk from master branch :)
16:55 proller joined #minetest-dev
17:00 Robert_Zenz joined #minetest-dev
17:19 H-H-H hmmm seems like the bah sourceforge is down so irrlicht d/s are down
17:19 nore_ joined #minetest-dev
17:19 harrison joined #minetest-dev
17:20 Puma_rc joined #minetest-dev
17:20 nrzkt down and up sometimes
17:20 nrzkt i will download it locally
17:25 proller joined #minetest-dev
17:25 H-H-H i have it in a git repo i could add to github if it helps
17:26 rubenwardy joined #minetest-dev
17:26 H-H-H sorry i meeant svn
17:27 nrzkt we could add it as a download on github minetest also
17:28 H-H-H would need to be maintained though
17:28 nrzkt yes
17:28 nrzkt http://irrlicht.sourceforge.net/ is down too
17:30 TBC_x shouldn't we use nullptr instead of NULL where possible?
17:30 nrzkt why ?
17:30 sfan5 why?
17:31 TBC_x if we'll migrate to C++11 eventually... it would work with smart pointers
17:31 sfan5 is smart pointers using auto everywhere?
17:31 sfan5 if thats the case, no
17:32 TBC_x smart pointers have specializet template for comparing nullptr
17:32 sfan5 sounds like a lot of overhead
17:33 TBC_x for compiler, maybe
17:33 TBC_x you can't initialize smart pointer with NULL
17:34 TBC_x I'm tracking the memory leak with smart pointers btw
17:35 sfan5 not being able to use NULL sounds like a stupid thing
17:36 TBC_x you'll get used to it, probably
17:36 sfan5 you know
17:36 sfan5 NULL has a point
17:36 sfan5 ah
17:36 sfan5 https://en.wikipedia.org/wiki/Smart_pointer
17:37 sfan5 it was those things
17:37 sfan5 answer: no
17:37 TBC_x reason?
17:37 sfan5 a lot of work
17:38 TBC_x replacing NULLs?
17:38 sfan5 no
17:38 TBC_x replacing raws?
17:38 sfan5 replacing every single mention of pointers
17:38 TBC_x could find someone to do it
17:39 sfan5 thats not the point
17:39 sfan5 it changes a lot of code
17:39 sfan5 causes merge conflicts with most if not all PRs
17:40 TBC_x could be done progressively
17:41 sfan5 still causes merge conflicts
17:42 TBC_x rebase
17:42 TBC_x needs coordinating
17:42 sfan5 lolno
17:43 sfan5 you won't get anyone to agree to rebase all the PRs
17:44 TBC_x true
17:45 TBC_x could change all pointers when refactoring tho
17:45 TBC_x as refactoring may cause merge conflicts anyway
17:46 sfan5 refactoring does not happen on all code at the same time
17:47 TBC_x rewriting for smart pointers usually requires to change a single line
17:48 sfan5 because only a single line of code uses the pointer types, right?
17:49 TBC_x not necessarily
17:50 TBC_x you can unwrap it if you need to use raw pointer
17:51 sfan5 that would be stupid
17:51 TBC_x yes
17:51 sfan5 then the majority of code would use raw points
17:51 sfan5 pointers*
17:51 sfan5 just to have less changed lines
17:54 TBC_x need to compe up with a better transition strategy before and if that happens
17:54 sfan5 or alternatively we just leave all the code alone and don't use smart pointers
17:56 TBC_x unless the code has significant memory leak and is hard to track down
17:57 sfan5 transitioning all code to smart pointers will cause more problems than fix memory leaks
17:57 TBC_x I'm aware of that
17:58 sfan5 why are you even trying to argue then
17:59 TBC_x majority of the problems would be just revealed by the smart pointers, not caused by them
18:00 TBC_x if you don't count needed refactoring as a problem
18:03 nrzkt last call: #2918 will be pushed in 30 min (android build failed due to sourceforge pb)
18:03 ShadowBot https://github.com/minetest/minetest/issues/2918 -- Kick players when shutting down server and fatal Lua stack exception occurs by nerzhul
18:11 hmmmm that's not a trivial PR
18:12 nrzkt i tell this 2 hours ago. hurry hup. And this was discussed with est31 and 30 min are okay :p
18:12 hmmmm where
18:12 hmmmm i don't see est31's approval anywhere
18:14 hmmmm you can't just push non-trivial commits without approval because you gave two hours notice
18:14 nrzkt we talked about the approach to do and it's approach to do.
18:14 sfan5 taking about PR does not suffice
18:14 hmmmm where is this conversation, i can't see it anywhere
18:14 nrzkt and it's a trivial fix :)
18:14 hmmmm it's not trivial.
18:14 sfan5 you need explicit approval
18:14 hmmmm this changes functionality, and in a bad way.
18:14 sfan5 and no it's not trivial
18:14 hmmmm in fact i'm about to -1 this commit because it's the wrong approach
18:15 hmmmm you shouldn't try to do anything non-trivial on crash
18:15 nrzkt okay then, what is good ? for a compat issue
18:15 nrzkt the good approach cannot be used with compat
18:15 hmmmm when an application crashes, it is in an undefined state where anything could possibly happen
18:15 nrzkt the good approach is to have a proper packet to sent to client and handle it. Here we use the compat way.
18:15 nrzkt this is not the final way, it's the compat 0.4 way
18:16 nrzkt here we don't handle minetest core crash
18:16 nrzkt the only path where the message is sent, if you read the PR correctly is when there is: Lua fatal error => exit and proper shutdown => exit
18:16 nrzkt not coredump
18:17 nrzkt coredump is not the PR goal.
18:17 hmmmm well
18:17 hmmmm I suppose this is okay
18:18 nrzkt and also, another proper thing we should add for future protocol is a TOSERVER_PING/TOCLIENT_PONG to handle the crash over Udp
18:18 hmmmm it's only handling uncaught exceptions, and not deeper bugs like memory corruption or memory access violations
18:18 leat3 joined #minetest-dev
18:18 hmmmm fine, +1
18:18 nrzkt thanks.
18:18 hmmmm but you can't just lie to us that you had approval from other people first
18:18 hmmmm that's a huge no-no
18:19 nrzkt this approach was discussed, then, i thought it's a go to code pr and review :p
18:19 hmmmm an idea is not the same as its implementation
18:19 nrzkt for coredump i dont think we could notify client without PING/PONG packets
18:20 hmmmm when a code review is done, the actual code written is reviewed as well as the approach taken
18:20 nrzkt PING/PONG can also permit to have timeouts clientside
18:22 hmmmm don't take this personally, but you are way too commit trigger-happy
18:22 hmmmm code isn't going to spontaneously combust if it isn't merged within a day or two
18:24 kahrl_ I thought there were already pings in the protocol
18:24 kahrl_ the lower-level one
18:24 kahrl_ are they not used?
18:25 nrzkt no problem :)
18:26 nrzkt kahrl_ : no atm if you server crash client stays ingame
18:26 sfan5 they are used
18:26 sfan5 but the client doesnt disconnect
18:26 nrzkt something is missing clientside then
18:26 sfan5 indeed
18:27 nrzkt i will push it now
18:27 leat3 joined #minetest-dev
18:28 nrzkt pushed
18:37 kahrl_ I see that Client::deletingPeer(timeout=true) is getting called in this situation
18:37 kahrl_ so all that needs to happen is that that method actually does something
18:37 leat3 joined #minetest-dev
18:41 MinetestForFun joined #minetest-dev
18:44 kahrl_ something as simple as https://gist.github.com/kahrl/f58cce9dd468045c8fa4 might work
18:44 kahrl_ (I've tested that it works in this situation, but not yet that it won't break other situations)
18:50 Gael-de-Sailly joined #minetest-dev
18:56 MinetestForFun joined #minetest-dev
18:58 TeTpaAka joined #minetest-dev
19:29 leat3 joined #minetest-dev
19:48 nore_ joined #minetest-dev
20:02 AnotherBrick joined #minetest-dev
20:05 blaise joined #minetest-dev
20:05 werwerwer joined #minetest-dev
21:10 nore_ joined #minetest-dev
21:11 proller joined #minetest-dev
21:19 kilbith joined #minetest-dev
21:19 kilbith left #minetest-dev
21:29 AnotherBrick joined #minetest-dev
21:48 AnotherBrick joined #minetest-dev
21:59 Routh joined #minetest-dev
22:13 leat3 joined #minetest-dev
22:25 Puma_rc joined #minetest-dev
22:36 SudoAptGetPlay joined #minetest-dev
22:52 crazyR would someone please document the tabheader[] formspec feild int he wiki please
23:05 leat3 joined #minetest-dev
23:22 exoplanet joined #minetest-dev
23:34 Sokomine joined #minetest-dev
23:35 Sokomine joined #minetest-dev
23:39 kilbith joined #minetest-dev
23:40 Sokomine joined #minetest-dev
23:42 Sokomine joined #minetest-dev
23:54 kilbith left #minetest-dev

| Channels | #minetest-dev index | Today | | Google Search | Plaintext