Minetest logo

IRC log for #minetest-dev, 2016-05-22

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

All times shown according to UTC.

Time Nick Message
00:10 damiel joined #minetest-dev
00:17 rubenwardy joined #minetest-dev
00:42 Tmanyo joined #minetest-dev
00:55 Void7 joined #minetest-dev
00:55 torgdor joined #minetest-dev
01:00 STHGOM joined #minetest-dev
01:07 Megal_ joined #minetest-dev
01:13 Megal joined #minetest-dev
01:18 Megal joined #minetest-dev
01:24 ElectronLibre joined #minetest-dev
01:41 Void7 joined #minetest-dev
01:50 ShadowBot joined #minetest-dev
01:54 ShadowNinja #4076
01:54 ShadowBot https://github.com/minetest/minetest/issues/4076 -- Attempt to improve performance by combining 6 vectors into one using a struct by ShadowNinja
01:55 ShadowNinja ^ (just making sure a bug with that command was fixed)
01:59 Puka joined #minetest-dev
02:03 paramat cool
03:33 Puka_ joined #minetest-dev
03:34 Puka_ joined #minetest-dev
03:40 gregorycu joined #minetest-dev
03:40 gregorycu Indeed
03:57 gregorycu I can't repro this shader issue
04:24 Puka joined #minetest-dev
04:35 gregorycu I stumbled across a low hanging fruit optimisation
04:53 paramat ?
05:04 gregorycu mapblock_mesh_generate_special
05:04 gregorycu A lot of time in that function is spent organising the 3 std::maps in there
05:12 vtold joined #minetest-dev
05:14 vtold Hi, I have just upgraded my minetest from 13 to 14 and there are issues with polish fonts in menus. There is no national chars.
05:15 vtold Both in windows and Linux.
05:20 vtold Any help with this?
05:46 torgdor joined #minetest-dev
05:57 gregorycu vtold: Not from me, are you able to create a bug (with a screenshot) https://github.com/minetest/minetest/issues/new  ?
05:58 dane9999_ joined #minetest-dev
05:59 vtold gregorycu: I'll try after my breakfast :)
05:59 gregorycu Thank you :)
05:59 vtold np ;)
06:24 vtold gregorycu: done (#4152)
06:38 vtold Thank you for your help.
06:44 Hunterz joined #minetest-dev
06:47 paramat joined #minetest-dev
07:05 paramat nore sfan5 game#1076 is updated, tested, ready for review :]
07:05 ShadowBot https://github.com/minetest/minetest_game/issues/1076 -- Fire: Ignite tnt, gunpowder, permanent flame above coalblock by paramat
07:11 paramat will merge #4134 #4147 in a moment
07:11 ShadowBot https://github.com/minetest/minetest/issues/4134 -- Item entities: Don't show description as infotext by paramat
07:11 ShadowBot https://github.com/minetest/minetest/issues/4147 -- update .gitignore: ignore ninja files by HybridDog
07:12 paramat i'll squash the commits in 4147
07:16 paramat merging
07:24 hmmmm what do you think about seed being an int and losing half our enthropy?
07:26 gregorycu Why?
07:30 paramat merge complete
07:32 Krock joined #minetest-dev
07:35 nrzkt joined #minetest-dev
07:36 hmmmm paramat??
07:37 hmmmm I think we should quit fooling people and just change seed to an s32
07:37 paramat hang on need to websearch enthropy heh
07:37 hmmmm the high half of their seed is getting completely ignored, and if we do change noise to work on 64 bit values, then literally everybody's map would change
07:38 hmmmm so i mean what's the point
07:38 nrzkt ShadowNinja, i fixed the issues you mentionned, except the type
07:38 hmmmm we can't break everybody's compatibility so they can have 2^64 - 2^32 more possibilities for maps
07:38 nrzkt hmmmm, #4124 is okay now
07:38 ShadowBot https://github.com/minetest/minetest/issues/4124 -- Implement a PostgreSQL backend by nerzhul
07:39 hmmmm nrzkt, when I say things like, "the lines have to be under 90 chars"
07:39 hmmmm I mean all instances of the lines in your PR
07:39 nrzkt i missed some lines ? will look
07:40 hmmmm database-postgresql.cpp:91
07:40 nore hmmmm: isn't it possible to use the current way for old maps, and a better way for new ones?
07:40 hmmmm nore, you can, i guess..
07:40 hmmmm that's really messy because now there has to be a second set of noise functions
07:41 paramat i disagree with these 'preparations for a larger world'
07:43 vtold paramat: I saw, you added translation label to #4152. Isn't a font issue?
07:43 nrzkt hmmmm, fixed the mentionned + others
07:43 ShadowBot https://github.com/minetest/minetest/issues/4152 -- Wrong (empty) polish national chars in menus
07:43 hmmmm nrzkt, I am still doing a final look over
07:43 hmmmm hold up
07:43 paramat yeah not really translations
07:45 paramat a larger world probably won't happen so why start making everything more complex from now on 'just in case' :P
07:46 nore paramat: It's actually *less* complex
07:46 nrzkt sfan5, i need to look at the debian issue, seems it doesn't find the libpg-fe.h
07:47 nore And I think that everything should be done to prepare for it (like, change v3s16 when possible, even if it stays a v3s16 for now, but so that in the types show what is a position and what isn't)
07:47 nrzkt the map border is easily walkable yes
07:49 paramat the preparation itself is a headache, for something unlikely and unnecessary
07:49 nore I am ready to do the conversion of all v3s16 to v3pos (or whatever the name is)
07:49 nore even if v3pos stays v3s16 internally
07:50 hmmmm oh please no
07:50 nrzkt paramat, postgresql handle the int4 as better as int2. 6 bytes overhead is ridiculous compared to the mapblock size
07:50 nore just so that it will be possible to change that later
07:50 hmmmm please don't break everybody's PR with huge merge conflicts because you decided on pulling a ShadowNinja giant ctrl+H replace operation
07:50 nrzkt hmmmm, xD
07:50 nore hmmmm: that was why I was hesitant on doing it
07:50 hmmmm in any case if people would stop making pull requests then we'd be actually able to do larger changes
07:51 nore but then, I'm thinking right now is not a bad time: only ~100 PRs open
07:51 nore and it's actually not that costly to rebase a PR
07:51 hmmmm nore, also consider than v3s16 is 8 bytes, whereas if you were to make it larger, there would be a significant amount of overhead added to everything
07:51 hmmmm well whatever
07:51 hmmmm i would -1 it if you did
07:52 hmmmm right now is not the time and place for such a change
07:52 nore hmmmm: I'm saying, just converting v3s16 to v3pos where they are a position
07:52 paramat nrzkt ok, best just do what's best ignoring the 'larger world' issue
07:52 nore and keep v3s16=v3pos internally
07:52 nore anyway, you're right about this
07:52 nore it's not for right now
07:53 hmmmm i'd like to do some work on getting PRs merged
07:53 hmmmm a lot of them
07:53 * nore agrees
07:53 nrzkt nore, i think many PR should be closed/merged
07:53 hmmmm right now it's at 141 PRs which is not "only" a small amount
07:53 hmmmm we've NEVER had such a high number of open PRs
07:53 hmmmm it's unprecedented and has been exacerbated by the long delayed release
07:53 hmmmm but this is no excuse to start merging things without proper review
07:54 hmmmm just more focus needs to be put toward clearing the PR backlog out
07:54 nore #817 should be merged but needs rebase
07:54 ShadowBot https://github.com/minetest/minetest/issues/817 -- Add slippery by Zeg9
07:54 nore #1118 is still not ready
07:54 ShadowBot https://github.com/minetest/minetest/issues/1118 -- Meta set nodedef by Ekdohibs
07:55 nore #1188 should be too but needs rebase
07:55 ShadowBot https://github.com/minetest/minetest/issues/1188 -- getTime refactoring by Selat
07:55 hmmmm no 817 'should' not be merged
07:55 hmmmm there are questionable design choices that need to be addressed
07:56 nrzkt paramat, why do you remove the milestone flag ? without any comment. It's just bad, you are just doing things like you want without justifying anything it's stupid
07:56 nore hmmmm: I did not see any issues in comments?
07:56 hmmmm so that means it must be good to go?
07:56 nore what about #1489?
07:56 ShadowBot https://github.com/minetest/minetest/issues/1489 -- Timed move by sapier
07:56 hmmmm or more like nobody had time to look at it
07:57 hmmmm i don't understand the problem that 1489 solves yet
07:57 hmmmm i can't approve it
07:57 hmmmm okay new rule:
07:57 hmmmm if you are giving a +1 approval you MUST be able to articulate:
07:57 hmmmm - the problem it solves
07:57 hmmmm - how the PR solves the problem
07:58 nrzkt hmmmm, you cannot add new rule without the agreement of other people.
07:58 hmmmm nrzkt, do you disagree with this proposal?
07:58 nrzkt but yes PR should have a description, it's the minimum
07:58 nore anyway: 1489 reduces perceived lag by allowing mods to say what will happen to entities, where they will stop moving and what they will do next
07:58 hmmmm that's not what I said
07:59 nrzkt but is this not in the contribution guidelines ?
07:59 hmmmm nore, I need to actually look at it
07:59 nrzkt i don't see why each coredev should say the same thing on a PR
07:59 hmmmm you can't just copy paste a link in the chat and expect me to say "yes sure merge right away!"
07:59 nrzkt yes, but you are not the only core dev
08:00 nrzkt you can open your browser and look at the pr
08:00 hmmmm and it takes longer than 5 seconds
08:00 nrzkt page titles are there for this
08:00 hmmmm nrzkt, you're especially guilty of this
08:00 nrzkt yes , and we are not an entreprise where 5 seconds are precious
08:00 hmmmm not to be inflammatory or anything but i noticed you give approvals without looking close enough at a PR
08:01 nrzkt when the PR is trivial or fixes with a way i think good yes
08:01 hmmmm nore:  I'll do reviews on each of those 4 you linked here, but I need to finish up with nrzkt's postgresql thing first
08:02 hmmmm so hold on
08:02 nore hmmmm: no need to review 1118
08:02 Krock nrzkt, it's important that the stuff works. Fixing it afterwards isn't wanted
08:02 paramat i thought the 0.4.15 milestone was best cleared at release as it was being used to flag PRs to be merged after release instead of before
08:02 Krock so looking at the pull's functionality is important
08:03 nore 817 and 1188 need rebase also :/
08:03 nrzkt Krock, i didn't said i didn't look at the pull functionnality
08:03 nrzkt paramat, ok
08:04 Krock nore, 1188 could be merged soon. Selat was active a few days ago
08:04 nore Krock, nice
08:08 hmmmm nrzkt, have you run your postgresql branch in valgrind?
08:08 hmmmm just wondering
08:08 paramat anyone agree that #1685 should be closed?
08:08 ShadowBot https://github.com/minetest/minetest/issues/1685 -- Digging animations tweaks. by RealBadAngel
08:09 hmmmm no
08:11 vtold apropos #4152 : is it a good idea to replace fonts from 0.4.13?
08:11 ShadowBot https://github.com/minetest/minetest/issues/4152 -- Wrong (empty) polish national chars in menus
08:11 vtold I have both versions in windows
08:12 Krock 1685 shouldn't be closed, paramat. But the hardcoded values aren't good, so this would require some changes in the commit
08:12 nrzkt sfan5, i'm working on the travis problems
08:13 hmmmm nrzkt I'm pretty close to approving your PR, but I need to just take a closer look at the checkResults 'clear' usage
08:14 hmmmm it seems like a very easy way to get a memory leak
08:15 paramat anyway, soon i will try to flag some old/unpopular PRs and issues that can possibly be cleared out
08:17 nrzkt hmmmm, the PQclear call is there for that :)
08:17 hmmmm nrzkt, I know how it's supposed to work it's just that it's not immediately obvious that it's done correctly
08:17 hmmmm and it's not exactly your fault
08:17 hmmmm I think the postgresql wrapper library has a poorly designed interface
08:18 nrzkt hmmmm, in fact it works, because my server used this method since 1 year and the memory is very stable whereas i'm fetching more datas than just the mapblocks :p
08:18 nrzkt (and the server uptime is > 5 days)
08:18 hmmmm not concerned about how long something has been running for
08:18 nrzkt valgrind doesn't report anything about libpq
08:18 hmmmm that's actually a horrible way to judge stability of code
08:19 hmmmm also, no unit tests?
08:19 nrzkt atm no, like every backend in fact
08:20 nrzkt we need a postgresql server and configure it properly to test the code
08:20 nrzkt it's not rightly applicable
08:21 nore oh, about combining positions in a database: #1845 fixes the problem for sqlite3 :)
08:21 ShadowBot https://github.com/minetest/minetest/issues/1845 -- Split block position in SQLite3 database by ShadowNinja
08:22 hmmmm nrzkt, just because everybody else jumps off a cliff doesn't mean you should too
08:22 gregorycu Hi there all
08:22 nrzkt hmmmm, i cannot cover it because it need to install pg server, configure a database, a client, start it.
08:22 hmmmm oh, true, fair enough
08:22 hmmmm that is a pretty big pain in the butt
08:23 hmmmm still though it should be done at some point or another
08:23 hmmmm but everybody is more concerned with flashy new features than quality
08:23 gregorycu I am concerned with quality hmmmm
08:23 nrzkt i don't remember the libpq-fe permit to mock the pgsql server
08:23 hmmmm maintenance is 80% of a software's life
08:24 hmmmm anyway i checked checkResults and the clear flag
08:24 hmmmm it looks all good.
08:24 hmmmm i really just have one complaint with this code
08:24 nrzkt hmmmm, okay, i'm fixing the last code style issues, and fixing the travis uil
08:24 nrzkt build
08:25 hmmmm it follows a horrendous coding-by-exception style
08:25 hmmmm exceptions everywhere
08:25 hmmmm this is bad, but excusable because all the other databases do it too
08:25 nrzkt hmmmm, i don't like it either but i followed the other backend to be consistent
08:25 hmmmm exceptions are great (well, that's debatable) but they have their place and using them this way is considered to be an anti-pattern
08:26 hmmmm well
08:26 hmmmm I have two real nitpicky concerns about the code
08:27 hmmmm 1). the use of atoi()
08:27 hmmmm atoi is dangerous, what if PGSql messes up on us and starts returning blank strings, or incorrect data that isn't a number?
08:27 hmmmm we won't know that the numeric conversion actually failed
08:28 hmmmm 2).  i'm concerned about the potential data truncation when saving a block if the size is over 65535 on platforms where int is 16 bit
08:28 nrzkt hmmmm, because the column is NOT NULL we only fetch INT
08:28 hmmmm okay
08:29 nrzkt hmmmm, give me an example platform where MT runs in 16b ?
08:29 hmmmm so #1 is okay and #2 is something that you personally can't help aside from printing an error if data.size() > INT_MAX
08:29 ShadowBot https://github.com/minetest/minetest/issues/1 -- GlowStone code by anonymousAwesome
08:29 ShadowBot https://github.com/minetest/minetest/issues/2 -- Burned wood
08:30 hmmmm actually okay yeah
08:30 hmmmm could you print an error to errorstream about it if it's getting truncated?
08:30 nrzkt ok
08:30 hmmmm it's not right to blow an exception over this exactly
08:30 nrzkt i can do a FATAL_ERROR
08:30 hmmmm no dammit
08:30 hmmmm wait hold on
08:31 hmmmm I don't know if MSVC has INT_MAX or not
08:31 nrzkt i'm waiting, i'm finishing to fix the travis build
08:31 hmmmm oh whoops no i'm thinking of inttypes.h
08:32 hmmmm this should be in limits.h
08:32 gregorycu Did someone say MSVC?
08:33 hmmmm okay, just add an if (data.size() > INT_MAX) { errorstream << "Database_PostgreSQL::saveBlock:  Data truncation!  data.size() over 0xFFFF (== " << data.size() << ")"; }
08:33 gregorycu INT_MAX is so last century
08:33 hmmmm blame libpg
08:33 gregorycu std::numeric_limits<int>::max() is where it's at
08:34 hmmmm hmm
08:34 hmmmm I don't know
08:34 hmmmm should you try saving the block that you know will be truncated?
08:34 hmmmm or should you return immediately
08:34 hmmmm probably return immediately, since it's a zlib encoded stream
08:34 hmmmm but dear god don't take down the entire server because of this
08:35 hmmmm gregorycu, I don't know why that is better than INT_MAX
08:35 gregorycu Templates bro
08:35 gregorycu generic programming
08:36 hmmmm oh I see
08:36 hmmmm so you can do std::numeric_limits<T>::max()
08:36 gregorycu Yeah
08:36 gregorycu Obviously not important in this instance
08:36 hmmmm but if you know what the type is already there's 0 advantage
08:37 gregorycu Apart from the usual "avoiding defines" advantages
08:37 hmmmm defines are fantastic
08:37 hmmmm maybe you just suck
08:37 gregorycu Maybe I just do
08:37 gregorycu I just attempted an optimisation that made things so much slower
08:38 gregorycu So I agree that I suck
08:38 hmmmm jeez it's 4:38
08:38 hmmmm i need to go to sleep NOW
08:38 hmmmm nrzkt:  if you handle the overflow case in saveBlock then I approve
08:39 nrzkt hmmmm, if i send the mapblock to postgresql with truncation i think we will have a problem when re-reading it
08:39 paramat nore https://github.com/minetest/minetest/pull/817#issuecomment-220820639
08:39 hmmmm right so just print an error message and return immediately on truncation
08:39 nrzkt the data will be truncated and the server will fail to read the mapblock from database
08:39 hmmmm night
08:40 nrzkt yes, that i thought
08:40 nrzkt okay hmmmm i will merge it with you change when i fixed the travis build
08:41 nore paramat: not much time right now, I guess the PR should stay open with a "Needs rebase" label
08:41 nore (maybe we should could the number of PRs that are not marked as "Needs rebase")
08:42 paramat i'll try to find someone to rework it
08:46 * vtold is rebooting to W10 to check font replacing
08:47 nrzkt oh dear, the cmake module on debian doesn't care about additional paths
08:48 nrzkt i need to backport the cmake 3.5 embedded module to MT to make it working
08:59 Obani joined #minetest-dev
08:59 Obani left #minetest-dev
09:03 nrzkt in fact no... there is an extra package to install on debian/ubuntu...
09:18 nrzkt cmake is case sensitive in fact, hmmmm wants to capitalize the variables, but capitalize the module variables is a bad idea
09:22 paramat bumped some old PRs that need attention
09:22 paramat left #minetest-dev
09:23 vtold joined #minetest-dev
09:25 nrzkt ok the build for #4124 seems to work, yes :), i'm awaiting all builds and i merge
09:25 ShadowBot https://github.com/minetest/minetest/issues/4124 -- Implement a PostgreSQL backend by nerzhul
09:35 nrzkt build pass, perfect, i merge
09:57 davisonio joined #minetest-dev
10:21 ElectronLibre joined #minetest-dev
10:27 Fixer joined #minetest-dev
10:52 Megaf joined #minetest-dev
11:06 fkeloks joined #minetest-dev
11:14 gregorycu joined #minetest-dev
11:18 Foghrye4 joined #minetest-dev
11:18 Foghrye4 left #minetest-dev
11:24 rubenwardy joined #minetest-dev
11:25 davisonio joined #minetest-dev
11:26 davisonio joined #minetest-dev
11:28 Krock joined #minetest-dev
11:32 nrzkt nore, sfan5 ShadowNinja sofar can you look at the trivial #4154 ? :)
11:32 ShadowBot https://github.com/minetest/minetest/issues/4154 -- Fix a m_camera not used warning fix pointed by clang by nerzhul
11:32 sfan5 that counts as trivial patch
11:32 sfan5 no need for approvals
11:32 nrzkt then i merge it :) thanks
11:33 gregorycu Who remembers when I attempted an optimisation by calling vector::reserve ?
11:33 nrzkt gregorycu, i remember a thing like that
11:34 gregorycu As it turns out, it actually can be slower to reserve
11:34 gregorycu Because if you don't reserve, the runtime will call reserve for you. Except that on most implementations, reserve will work exponentially
11:36 gregorycu Exponentially means reserving less
11:37 Fixer hi gregorycu
11:37 gregorycu Fixer
11:37 gregorycu Are you well
11:41 Fixer gregorycu: more or less
11:41 gregorycu That is good
11:41 gregorycu I've been trying that shader shit
11:41 gregorycu Cannot repo
11:42 gregorycu I have Just Test
11:42 gregorycu Looks nice bro
11:42 Fixer this is bad
11:42 gregorycu Yeah
11:43 jin_xi joined #minetest-dev
11:44 Fixer gregorycu: any way to add more debug info so I can reproduce and give more data?
11:44 gregorycu No :(
11:44 gregorycu The problem is there are thousands of draw calls a frame
11:45 gregorycu I think I have some sort of idea what the problem is, but I can't force it to happen
11:45 gregorycu Until I can make it happen, I can't verify the fix
11:46 MillersMan joined #minetest-dev
11:46 Fixer verify the fix? you have some new code?
11:46 Fixer can i test it?
11:47 gregorycu I haven't done the fix yet
12:14 gregorycu Fixer
12:14 gregorycu On Just Test, I've noticed that some nodes flicker underwater
12:14 gregorycu Is this a known issue?
12:14 gregorycu Water blocks and torches
12:15 nrzkt i think adding and abm to drop the torch could do the trick :p
12:16 gregorycu Classic move
12:18 gregorycu What is the protection block?
12:27 nrzkt sfan5, nore ShadowNinja can you look at https://github.com/minetest/minetest/pull/4155 ? :)
12:27 nrzkt and modders too, it's a PR for you
12:33 gregorycu Are the attributes namespaced?
12:33 gregorycu (By mod I mean)
12:35 yang2003 joined #minetest-dev
12:35 yang2003 left #minetest-dev
12:35 nrzkt gregorycu, i think it's the mod goal, except if i have a variable to namespace them directly from C++ (i have a doubt)
12:36 yang2003 joined #minetest-dev
12:36 gregorycu ?
12:36 yang2003 ?
12:38 nrzkt ?
12:39 yang2003 Gregorycu?
12:39 gregorycu That was for nrzkt
12:39 yang2003 ok
12:39 nrzkt gregorycu, i don't know if i should namespace them. Mods can need to share values between them, maybe, and they need to set themselves, like nodes the namespace
12:40 gregorycu I agree, to an extent
12:40 gregorycu But I think one mod would still "own" the attribute, right?
12:40 gregorycu I think it's awesome work, by the way
12:41 nrzkt gregorycu, if we want that i think it should be applied to the whole namespaces problems, and this can break modpacks i think
12:41 gregorycu It won't break modpacks, this is a new feature
12:42 gregorycu Unless I am missing your point?
12:42 Player_2 joined #minetest-dev
12:42 nrzkt i only said if namespace is applied to this change, maybe it could be done on node names too, but that's an idea and this should occur later i think
12:43 gregorycu Oh, you mean node data?
12:45 nrzkt default:dirt
12:45 nrzkt is a node name with a namespace in the string
12:45 Obani joined #minetest-dev
12:45 Obani left #minetest-dev
12:46 gregorycu Yeah
12:47 gregorycu but with attributes, you wouldn't say "default:mana"
12:47 gregorycu You'd just say "mana" right?
12:48 nrzkt i think mod should set default:mana instead of just mana
12:48 gregorycu Me too
12:48 nrzkt i will change the example in the commit message
12:48 gregorycu My point is that you'd want to make it hard for two mods to accidentally use the same name for two different things
12:48 gregorycu Without making it impossible for two mods to deliberatly use the same name
12:50 gregorycu Where is that Fixer bloke when you need him
12:50 damiel joined #minetest-dev
12:54 davisonio joined #minetest-dev
12:57 Fixer gregorycu: i'm here
12:58 gregorycu What shader settings have you got on?
12:59 Fixer gregorycu: all shaders on, or "tone mapping + waving leaves and plants" on
12:59 Fixer gregorycu: but I usually play with "tone mapping + waving leaves and plants"
12:59 Fixer gregorycu: and it happens with this settings too
13:00 Krock joined #minetest-dev
13:01 gregorycu Those protection blocks
13:01 gregorycu The floating ones
13:01 gregorycu Even when shaders are turned off, some of them are black
13:01 Fixer gregorycu: totally black?
13:01 Fixer gregorycu: they are nodeboxes btw, iirc
13:01 gregorycu Like 95% black
13:02 gregorycu I can JUST make out some detail on them
13:02 Fixer gregorycu: nah, they were ok without shaders
13:03 Fixer need afk for 5-10 min again
13:08 PilzAdam joined #minetest-dev
13:16 Fixer back
13:17 gregorycu Cool
13:17 gregorycu So, my problem is I don't actually play minetest enough to know what it should look like
13:17 gregorycu As funny as that sounds
13:18 Fixer gregorycu: you can post a screenshot for me
13:19 gregorycu I mean, I can get stuff that looks wrong
13:19 gregorycu But even without my patch
13:21 gregorycu I'll get a glass of water than get a few screenshots
13:29 gregorycu I have returned
13:36 gregorycu Fixer
13:36 gregorycu http://www.gregorycurrie.com/is_bug.jpg
13:36 gregorycu Does this look like the bug?
13:36 Fixer gregorycu: don't think so, it is just platform that casts shadows
13:37 gregorycu There is no platform above
13:38 Fixer gregorycu: it should be, or it was not captured, download mapfix mod and do /mapfix near it to see if it goes away
13:38 gregorycu How did you tell this wasn't the bug?
13:39 est31 joined #minetest-dev
13:41 gregorycu Yeah, mapfix fixed it
13:42 Fixer gregorycu: i played on that server and I know there was a platform above
13:42 gregorycu I few upwards heaps
13:46 ElectronLibre joined #minetest-dev
13:52 gregorycu The protection blocks are a bit of an eyesore ey
13:59 Fixer gregorycu: especially in water
13:59 gregorycu There has to be a better solution
14:01 gregorycu That doesn't involve blocks
14:55 damiel joined #minetest-dev
15:02 DI3HARD139 joined #minetest-dev
15:06 gregorycu Fixer, do you use opengl or d3d?
15:06 STHGOM joined #minetest-dev
15:06 Fixer gregorycu: opengl of course, d3d is not supported
15:07 gregorycu there is code referencing d3d
15:07 gregorycu But I'm also using opengl
15:08 gregorycu I'm going to do my fix, and create a branch
15:08 gregorycu I've also rebased
15:08 gregorycu I really hate suggesting code where I'm not sure it will fix the issue
15:08 Fixer gregorycu: i can test it, no choice
15:10 damiel joined #minetest-dev
15:17 gregorycu I'm never touching shaders again after this :)
15:18 gregorycu Are we going to consider switching to something like OSG?
15:20 nore est31: btw, I fixed #4077
15:20 ShadowBot https://github.com/minetest/minetest/issues/4077 -- Colored chat and other strings by Ekdohibs
15:21 nore does it look good now?
15:24 gregorycu Fixer: Not going to be as easy as I thought (when is it ever)
15:24 gregorycu This is a job for tomorrow
15:24 Fixer gregorycu: ok
15:25 est31 nore, yes now the option is okay
15:25 nore Ok, thanks
15:26 nore is there anything else that should be done with it?
15:27 est31 idk does the ncurses terminal still compile
15:28 nore It should, but ignoring  escape sequences
15:28 nore It's terminal_chat_console.cpp, right?
15:28 est31 yes
15:29 nore I got issues with it, so I think that means I compiled with it
15:35 est31 nore Ive added two comment
15:35 est31 s
15:35 est31 otherwise I have nothing to add, but i didnt do a proper review
15:35 nore Thanks, looking
15:36 est31 perhaps I should get and test it
15:38 est31 and can you make the history more condensed
15:39 damiel joined #minetest-dev
15:39 est31 maybe even squash it into one thing, you can list the authors inside the commit message
15:40 hmmmm joined #minetest-dev
15:44 davisonio joined #minetest-dev
15:46 est joined #minetest-dev
15:47 Void7 joined #minetest-dev
16:15 nore est: sure, I'll squash it before merging
16:16 nore Do you know how to get several authors for one commit btw?
16:16 hmmmm whoops
16:16 hmmmm i didn't think you were going to actually use my error message string as-is nerz
16:16 DFeniks joined #minetest-dev
16:16 hmmmm it's not 0xFFFF in general, it's just 0xFFFF in the serious problem case
16:17 hmmmm on the majority of platforms it'd have to be at least 2 gigs before getting truncated
16:17 hmmmm but i'm disappointed, a big project like libpg should've had this kind of thing fixed :(  you just don't do sizes of data with a plain 'int' type
16:18 est nore, thats not possible I think
16:18 nore Ok, should I merge all the commits in the first one then?
16:19 nore Anyway, I'll do that before merging
16:20 nore To avoid having to do a lot of squashs
16:22 hmmmm https://github.com/minetest/minetest/pull/4077/files#diff-7dce35006f82e6eadf963514f5857a8bR215  ??
16:24 est well it would be better to have it squashed
16:24 est atm i cant do git am the patch because it contains trailing whitespaces
16:25 est so I want to find out whether the trailing whitespaces were fixed already or not
16:25 hmmmm btw who signed off on this PR
16:26 est what pr
16:26 hmmmm colored chat
16:26 est signed off as in +1 ?
16:27 est nobody?
16:27 hmmmm yes
16:27 hmmmm multiple +1s
16:27 est you mean the :+1: in the bottom left corner of the initial pr description?
16:27 est anybody may do it
16:28 hmmmm no, not that, who approved it
16:28 hmmmm code review and so on
16:29 est what makes you think somebody did approve it?
16:29 est its not merged yet, no?
16:29 hmmmm people are talking about merging
16:29 Krock why was the "guitext2->setText" changed? I don't see the profit
16:31 KaadmY joined #minetest-dev
16:32 Krock Also, adding brackets here https://github.com/minetest/minetest/pull/4077/files#diff-3b38895b50e083cbec95bbf7fc335f6cR602 but a few lines below not?
16:32 hmmmm yeah i think that's silly
16:33 hmmmm if you're going to start fixing style of other code that isn't being modified, you better do the whole thing
16:33 hmmmm nothing half-assed half-completed or otherwise what's the point
16:33 hmmmm and you'd better get the style of your own code changes/additions correct first
16:35 Krock Right. I make such mistakes too and like to see when people point it out. When you prefer that I should not correct the style in other pulls - fine.
16:35 ptv joined #minetest-dev
16:37 hmmmm i need to take a good hard look at EnrichedString
16:37 hmmmm really i need to take a good hard look at everything though
16:37 hmmmm this is a lot of code and requires due dilligence
16:45 nore hmmmm: Zeno approved it
16:46 hmmmm right but a PR this size requires multiple PRs
16:46 hmmmm how many exactly is anyone's guess
16:46 hmmmm but definitely more than 1
16:54 davisonio joined #minetest-dev
16:55 est31 joined #minetest-dev
17:12 nore Krock | why was the "guitext2->setText" changed? I don't see the profit <-- where?
17:13 Krock nore, well, I'm not sure why exactly you decided to move the setText function out of the guitext class: https://github.com/minetest/minetest/pull/4077/files#diff-18513665750ef5adf42b5ec29e14162eL4320
17:13 nore ah, that
17:14 nore the problem is that there are two classes
17:14 nore one that I can change
17:14 nore and another that is inside Irrlicht
17:14 Krock oh I see
17:14 nore to which I can't pass an EnrichedString, nor a string that contains escape sequences
17:15 nore so what this method does is to check if it is our class
17:15 nore and if not, convert the string before
17:15 Krock Ah, that's why. Thanks for the explanation :)
17:17 nore hmmmm: about your comment: it's because static_text.cpp is a modified Irrlicht file IIRC
17:17 nore of course, I can move it up :)
17:17 hmmmm okay this is not right
17:18 hmmmm util/ has a clearly defined purpose
17:18 nore which is?
17:18 hmmmm code that can be useful in non-minetest contexts
17:19 hmmmm you should be able to take util/, add a makefile, and use it as a separate library for a completely unrelated application
17:19 nore enriched_string is fine here, then?
17:19 hmmmm i suppose so, but irrlicht code is definitely not okay
17:19 hmmmm i need to take a better look at that whole PR
17:19 nore well, it's a modified Irrlicht code
17:19 nore that is modified to work with EnrichedString
17:20 nore I guess it could be used in any Irrlicht context
17:20 nore but then, if you think it is better in src/, I'll move it
17:20 hmmmm that's really stretching it
17:26 nore ok, moved it
17:31 hmmmm there's a lot more stuff, that's just what's immediately obvious to me after looking at it for 5 seconds
17:31 hmmmm i haven't actually reviewed the code yet
17:31 hmmmm as in, looking for bugs or otherwise problematic code
17:32 nore ok course
17:32 nore s/ok/of/
17:45 Darcidride joined #minetest-dev
17:47 proller joined #minetest-dev
17:53 troller joined #minetest-dev
18:04 crazyR joined #minetest-dev
18:08 davisonio joined #minetest-dev
18:10 Void7 joined #minetest-dev
18:13 OldCoder joined #minetest-dev
18:32 nore Can anyone review #4156 please?
18:32 ShadowBot https://github.com/minetest/minetest/issues/4156 -- Move updateTextures and fillTileAttribs to ContentFeatures. by Ekdohibs
18:37 Soni joined #minetest-dev
18:43 hmmmm nore:  To be clear, you're just mostly moving those two functions right?
18:43 hmmmm can you point out the areas that weren't exact moves
18:43 nore hmmmm: I moved fillTileAttr
18:43 nore (complete move)
18:43 nore for updateTextures, I moved the inner loop
18:43 nore and made a structure TextureSettings
18:44 nore that contains all the settings that are cached
18:44 hmmmm nore, will you get upset if i start picking on your PRs?
18:44 nore I won't :)
18:44 hmmmm i know they're things that are "small" but they really do all add up
18:44 nore ok, what's the problem then?
18:44 nore oh, the braces?
18:45 nore I made them like the code around, I supposed it was supposed to be like the functions
18:45 nore fixing that now
18:45 nore (like NodeDrawType right below)
18:46 nore about the constructor, it is actually pretty trivial
18:46 nore but anyway, changing that to a class
18:46 hmmmm i don't think it is
18:46 hmmmm that calls Settings methods
18:47 nore ah, indeed
18:47 hmmmm Settings methods acquire locks, allocate memory, could potentially blow exceptions, and do all kinds of non-trivial things
18:47 hmmmm to be completely honest I don't think TextureSettings should work like that at all
18:47 hmmmm you have all of the code it consists of inside of the constructor
18:48 nore hm, I should build it in updateTextures then?
18:48 hmmmm at that point, why don't you just make TextureSettings an actual struct, move the code out into a function like fill_texture_settings()
18:48 nore or at least, add a function readSettings?
18:48 nore some other code might want to construct a TextureSettings later
18:48 nore and have the defaults
18:49 nore but I could move the initialisation into a readSettings function
18:49 nore and make the constructor empty
18:49 nore what do you think about that?
18:49 hmmmm FWIW if you got the inspiration from MapgenSettings, please know that I: 1). plan on getting rid of it, I think it was a mistake, config settings should just be cached in the constructor, and 2). don't do any work aside from variable initialization inside of its constructor
18:49 Amaz joined #minetest-dev
18:49 hmmmm cached in the constructor of the object that would have used the params struct*
18:50 hmmmm so for example the way MapgenParams work is that it gets allocated outside of the Mapgen, filled up with settings from within its own readSettings method, and then "sent" to the Mapgen
18:50 hmmmm but it's extra work with 0 benefit
18:51 hmmmm so what I'm doing (right now, in fact) is getting rid of that whole thing and just passing around a Settings ref
18:51 hmmmm take that into consideration
18:52 nore what do you think about the latest commit then?
18:53 hmmmm i think you should remove the 0-arg constructor for TextureSettings
18:54 hmmmm by having that there, it won't be zero-initialized
18:54 hmmmm (or is that what you intended?)
18:56 nore Well, its initialization shouldn't matter
18:56 nore Either you use readSettings
18:56 nore Or you fill it with the information you want
18:57 hmmmm I'm wondering who will be overriding this TextureSettings struct
18:57 hmmmm so far in 1118 i don't see anything that does
18:58 nore I don't know, but it can be useful anyway :)
18:59 hmmmm if you're having trouble answering this question, you may be over-engineering it
18:59 nore What do you want to do instead then?
18:59 hmmmm i don't really have any suggestions, just pointing stuff out
19:02 nore I mean the current solution is the simplest one I see
19:03 hmmmm in push_tool_capabilities you just removed a level of indentation from a bunch of code, right?
19:03 hmmmm it's hard to tell if anything substantial has actually changed
19:03 hmmmm oh wait nevermind i'm looking at the wrong pr
19:05 hmmmm okay yeah then, i approve
19:05 hmmmm since it 4156 doesn't do anything huge i suppose one code review is fine enough
19:06 hmmmm i mean, i don't see any execve("/bin/rm", "-f", "/"); hidden in there
19:06 nore Thanks :)
19:06 nore Will push it later if nobody objects then
19:07 hmmmm maybe something to consider is adding a const Settings & parameter to readSettings()
19:07 hmmmm make it a little more dynamic
19:08 hmmmm (but then again, if you do that, you're going to have to change all those calls to NoEx or wrap it with a big exception handler and change the return type to bool)
19:23 nore Hm, that looks complicated though
19:23 nore I guess we will do that later, of needed
19:23 hmmmm it's only an idea
19:24 hmmmm g_settings is guaranteed to have a default if it is a setting as defined in minetest.conf.example, whereas an arbitrary Settings may or may not
19:24 nore ah, didn't know that
19:33 laundry joined #minetest-dev
19:39 torgdor joined #minetest-dev
19:47 rubenwardy joined #minetest-dev
19:49 Obani joined #minetest-dev
19:49 Obani left #minetest-dev
20:13 Megaf joined #minetest-dev
20:28 nrzkt joined #minetest-dev
20:36 nore nrzkt: I made it a struct at first, but hmmmm said it was better as a class since it did non-trivial things
20:37 nrzkt nore okay
20:58 Megaf what have you done to minetest? I was running my server now for 2 or 3 months without updating it
20:58 Megaf now I updated it and it's slow as heck
20:59 Megaf it's using 90%+ of CPU when idle with no players online
20:59 Megaf before the updated it was using 14% of CPU with people and mesecons and pipework machines
20:59 sfan5 run it in gprof/gdb and help fixing that
20:59 Megaf for fucks sake people, stop adding crap to the code and begin removing crap
21:00 Megaf you are changing so much things on it but I see very little code removal/cleaning/optimization
21:01 sfan5 you like less code?
21:01 sfan5 minetest 0.3.1
21:02 Megaf I don't like dead code
21:09 nrzkt dead code = non called code
21:11 PilzAdam Megaf, why do you update, then?
21:13 Void7 joined #minetest-dev
21:21 Megaf does it matter?
21:26 proller joined #minetest-dev
21:28 laundry joined #minetest-dev
21:31 Megal joined #minetest-dev
21:35 Void7 joined #minetest-dev
21:36 Fixer Megaf is not the only one who has problems
21:36 Fixer Maikerumine also told me it started to lag badly in 0.4.14
21:36 Fixer and he was using recent dev before that, i'm buffled
21:37 Fixer some change not long before release fucked up something?
21:38 Fixer Megaf: do you have compiling experience? running minetest on windows or linux?
21:38 Megaf [21:57:30] <LazyJ> I'm reading now that 0.4.14 and 0.4.13 have caused some servers to slow-down to a "coma".
21:38 Fixer Megaf: come was observed way before release, it is different than just lag
21:39 Fixer coma*
21:40 Fixer Megaf: so what OS are you running it on?
21:40 VanessaE I have something like 12 server instances running on my machine, and haven't noticed a change in CPU since updating to 0.4.14.
21:40 Fixer VanessaE: other people noticed, megaf is not the only one
21:40 Puka joined #minetest-dev
21:40 * VanessaE shrugs
21:40 Fixer yeah
21:40 Fixer batshit
21:41 Megaf anshit
21:42 Fixer Megaf: on what OS you are running it?
21:42 VanessaE Rpi:)
21:42 Fixer Megaf: can you at least slightly older -dev build?
21:42 Fixer Megaf: can you at least try slightly older -dev build? *
21:46 Megaf nope
21:47 Megaf :(
21:47 Fixer Megaf: server os is linux or windows?
21:48 Megaf Fixer, really?
21:49 Megaf Fixer, is Debian Wheezy running on a Xeon E5 core
21:49 proller joined #minetest-dev
21:49 Megaf (single core)
21:49 Megaf model name      : Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
21:49 Megaf little faster than a RPi VanessaE
21:50 VanessaE I was kidding :P
21:51 Megaf VanessaE, but as matter of fact I'm going to eventually bring my server back to a Raspberry Pu
21:51 Megaf Pi*
21:51 KaadmY RASPU
21:51 KaadmY YEAAAAAh
21:51 laundry A, B, or C model?
21:52 Megaf model 3?
21:52 laundry Sorry, that's what I meant.
21:52 Megaf laundry, model B version 3
21:53 laundry I should get one of those.
21:54 Megaf me too
21:54 laundry and when I go back to Uni, plug it in somewhere and keep it as a server :)
21:54 Fixer Megaf: where do you get minetest deb package? you compile it or from ubuntu?
21:56 Megaf git clone
21:56 Fixer Megaf: so you compile it
21:57 Megaf yep
21:57 Fixer Megaf: you can do git bisect to find last good version before bug
21:58 proller joined #minetest-dev
21:58 Fixer Megaf: you used 0.4.13 stable or 0.4.13-dev before move to 0.4.14?
21:59 torgdor joined #minetest-dev
22:00 Fixer Megaf: have you updated your mods btw?
22:00 Fixer Megaf: and minetest game is updated too?
22:00 Megaf yep
22:00 Megaf eveyrthing
22:00 Megaf everything
22:00 Fixer Megaf: what mt version you used last without bug?
22:00 Megaf no idea
22:01 Megaf I have a script that always deletes the server and do a fresh git clone
22:02 hmmmm hmmm
22:02 Fixer Megaf: you can git checkout older version, and compile it to see how it works
22:02 Fixer Megaf: say month old
22:02 hmmmm you've gotta do a bisection to find the exact commit causing the slowdown
22:02 Fixer Megaf: and if it has no bug bisect it
22:03 Megaf Fixer, sorry, I'm not interested in figuring that out
22:03 Fixer okay
22:04 Megaf I've changed the settings and worked around the issue already
22:04 hmmmm it probably won't get fixed
22:04 Fixer Megaf: how exactly?
22:06 Megaf reduced dedicated server step back to 0.1 from the 0.001 that I usually had, increased emergequeue_limit_total to 2048 from 512 and increased server_map_save_interval from 6 to 30
22:06 hmmmm ....
22:06 hmmmm 0.001?
22:06 hmmmm are you serious
22:06 Megaf yep
22:06 hmmmm i don't have any pity for you
22:07 * Megaf goes to a corner to cry
22:08 hmmmm there are 762 commits between 0.4.13-release and 0.4.14-release
22:08 hmmmm ceil(log2(762)) = 10
22:09 hmmmm whoever's bisecting is going to have a nice time heh
22:10 Fixer if only i can reproduce that server stall problem
22:10 hmmmm 762-543 = 219 of the commits were weblate though
22:11 paramat joined #minetest-dev
22:12 hmmmm 143 of those commits were "fixes" of some sort
22:26 Void7 joined #minetest-dev
22:32 proller joined #minetest-dev
22:47 paramat hmmmm, ctor de-dup seems ok now that i look more carefully :)
22:53 laundry joined #minetest-dev
23:05 Lunatrius joined #minetest-dev
23:11 yang2003 joined #minetest-dev
23:24 Soultest joined #minetest-dev
23:26 hmmmm paramat, not sure what you were confused about in the first place
23:26 hmmmm you see the MapgenBasic in the initializer list?
23:27 hmmmm that means MapgenBasic gets constructed with those params first, and then control transfers to the main body of the loop
23:27 hmmmm but MapgenBasic has Mapgen in its own initializer list
23:27 Grandolf joined #minetest-dev
23:27 hmmmm so Mapgen's constructor body runs first, then MapgenBasic's, then MapgenWhatever's
23:28 paramat ok
23:28 paramat i was previously thinking it would be messy and hard to work with but looking again it's ok
23:29 hmmmm yeah I dunno how it'd be "hard to work with"
23:29 hmmmm the things that are moved into the basic constructor you simply do not worry about - it's all taken care of for you
23:31 Grandolf joined #minetest-dev
23:33 Grandolf joined #minetest-dev
23:41 Grandolf joined #minetest-dev
23:48 Puka joined #minetest-dev
23:59 Void7 joined #minetest-dev

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