Present: hoijui, abma (late), zerver, jK, Kloot, Tobi
__ Agenda _____________________________________
- Welcome
- Release plan
- Who wants to do what in big refactor?
Welcome
<Tobi> hey
<hoijui> hello
<hoijui> hehe :D
<hoijui> zerver new rank :D
<zerver> :)
<hoijui> guess we should start
Release plan
<hoijui> about a .8:
<hoijui> last week, we decided to make a .8 with bump water shader fixes, the path finder fix for S44, and jk's shadow space linearization (or something like that
)<zerver> ok
<hoijui> i ported them to the release branch, but the bunp water stuff .. i somehow did it wrong
<hoijui> i forgot what exactly.. but i think it hangs GML build or something
<zerver> u want me to look at it?
<hoijui> or maybe the one who made the commits in the first place...
<hoijui> that would be kloot
<hoijui> i did not upload my ported comits yet, i will do so on my own github repo..
<Kloot> I cannot debug gml builds on my current box
<hoijui> ok
<Kloot> but I assume it's a deadlock
<hoijui> yeah
<zerver> oh yeah, i remember some of your commits adding some locks recently
<hoijui> so it dead-locks in master too?
<zerver> adding locks is risky business
<Kloot> more riskier than it should be tbh
<zerver> it probably does lock in master too
<hoijui> https://github.com/hoijui/spring/commits/tmp
<zerver> lua is the reason why locking is such a nightmare in spring
<hoijui> the last 4 commits on that branch (based on current release branch)
<hoijui> should we discuss lua mutli state again?
<zerver> actually, in my lua split branch, there are some improvements to locking
<zerver> i made a debugger to analyze the locking order and find problem points
<hoijui> wel..
<zerver> and then tried to make the lua mutexes be the one that always lock first
<hoijui> lets move that to end of meeting? or make new point?
<hoijui> btw.. jk and tobi, you here?
<jK> sure
<Kloot> afaict in this there is only ever one mutex involved when creating & reinstancing a water renderer
<hoijui> ok
<Kloot> +case
<hoijui> anythign else to say about 0.82.8.0?
<Kloot> lua doesn't enter into it
<zerver> yeah, but what about opengl calls from the wrong thread?
<hoijui> i guess it is most likely somethign i did wrong when fixing cherry-pick conflicts
<zerver> 82.x does not have glShareLists enabled
<Kloot> doesn't matter either here
<Kloot> only difference is that GL calls from the destructor are now in another thread
<Kloot> but the ctor also makes them
<zerver> that does make a difference
<Kloot> hoijui: can you test master then?
<hoijui> ah yeah, can do that
<hoijui> i'll report back to you when i did that
<hoijui> can we go on then?
<Kloot> k
main points:
- hoijui will test bump-water changes in master (and maybe temporary release branch)
- kloot and/or zerver will try to fix stuff
Who wants to do what in big refactor?
<hoijui> i already started to do some stuff with projectiles
<hoijui> moving rendering code to separate files
<zerver> about previous point: any ogl calls from sim thread (not issued by lua) can hang spring
<hoijui> .. just saying so.. nobody else would do big changes there, or coordinate with me
<zerver> did no start any work wrt big refactor
<zerver> and this idea i had earlier about big refactor and possible threading of sim by disallowing all object to directly modify themselves wont work i think
<zerver> and the reason is... guess what... lua
<hoijui> what i am doing is basically, moving the Draw() and DrawOnMinimap() into separate files
<hoijui>
<zerver> it is kind of hard to disallow lua to directly modify itself
<hoijui> well.. as a first step, what i change is only going to change something at compile time, really
<Kloot> are you just moving the code into separate files, or also into separate classes?
<hoijui> also separate classes
<hoijui> but the methods are called in the same places, wiht the same effect
<hoijui> i'll upload to a branch on my repo too
<Kloot> so p->DrawOnMinimap(*lines, *points); becomes drawp->DrawOnMinimap(*lines, *points); and drawp stores p?
<hoijui> "make -j4 all" on spring really brings my PC down!
<hoijui> 4 core machine
<hoijui> it becomes:
<hoijui> pOnMinimapDrawer->Render(*p)
<hoijui> while lines and points are handed over to the drawer before that
<zerver> actually it just loads the cpus, machine only becomes unresponsive for real when there is heavy disk access also
<hoijui> theres also pDrawer->Redner(*p)
<hoijui> Render*
<Kloot> ah that works too
<jK> it would be much smarter w/o dynamic casting
<jK> -smarter +faster
<hoijui> cants see how it makes a difference really
<hoijui> its like cast + 100+ lines of code which are all much heavier
<jK> meh gamedev is down
<jK> they have a article about it
<jK> dynamic casting is a problem
<hoijui> yeah.. if there is a nicer way... sure
<hoijui> that is.. dynamic_cast?
<jK> spring uses it ways too often
<hoijui> i dont use that one
<Kloot> for weapon projectiles you would not even need to dyncast, just a switch on WeaponProjectile::projectileType
<jK> dynamic casting can also be indirect via class inherit
<zerver> dynamic_cast is very slow yes
<Tobi> any numbers?
<jK> gamedev is down :<
<jK> their article has numbers
<zerver> if you look in object.h i made some attempt at a faster method
<hoijui> as said, i dont use that
<hoijui> i just cast a pointer
<hoijui> the branch: https://github.com/hoijui/spring/commit ... Separation
<hoijui> the most interesting commit:
<hoijui> https://github.com/hoijui/spring/commit ... Separation
<hoijui> arg
<hoijui> https://github.com/hoijui/spring/commit ... dea5dc80de
<hoijui> this one
<hoijui> this is where i cast:
<hoijui> https://github.com/hoijui/spring/commit ... 80de#L5R11
<Tobi> hmm so only static casts
<jK> virtual functions aren't static
<Tobi> they aren't casts either
<jK> they are something similar
<jK> nearly the same
<jK> from the ops needed
<zerver> they have the same performance impact at least
<Tobi> so then there is no performance need to avoid dynamic casts <_<
<zerver> nope
<hoijui> you say, we should not use virtual functions either?
<zerver> if we need performance, no
<hoijui> i can't imagine that this really matters
<Tobi> IMO, what you want first is correctness and good design, once you got that you can optimize the bottlenecks
<hoijui> its like the inlineing thing
<hoijui> inlineing a 40 lines functions makes no sense
<hoijui> yeah
<zerver> it only matters if it is called often
<hoijui> i agree with tobi
<zerver> performance difference is about 10 times
<hoijui> and as we use C++, we should use OO, and i cant imagine nice OO design without inheritance
<hoijui> that is not what matters, it matters in relation to the code inside the function
<zerver> number of lines is not the key point
<hoijui> it is a godo estimate
<hoijui> good*
<hoijui> i know it is not linear to that
<zerver> the key point is if the function call needs data to be copied etc, which could be avoided by making it inline
<hoijui> i am pretty sure that there is some optimization at runtime
<hoijui> i mean.. Java is pretty fast by now, and it has all functions virtual, plus it allows t change class structure at runtime, which C++ does not. so C++ can be optimized even better
<zerver> sure, compiler makes lots of stuff inline without asking
<hoijui> i mean, "cacheing" which function is used
<hoijui> mm ok maybe not doing that
<hoijui> but what now...
<hoijui> should we have a vote? or .. jk.. are you going to find reference? or do you agree with what Tobi said?
<hoijui> i dont want to discuss low level shit for hours now
<jK> meh can't find the article with their changed design
<hoijui> you cant remember anything?
[22:42:34] ** Kloot left the channel( Ghosted ).
[22:42:35] ** Kloot joined the channel.
<zerver> i know from testing that dynamic cast is 10 times slower than some custom method, but i dont think we should avoid dynamic casts
<hoijui> ok
<hoijui> i did not do this in master, or merge it there yet so we can discus it first, and in case we need to do fixes in that code that woudl have ot go to 0.82 still
<hoijui> but once everyone is ok with it, i would like to put it in master
<Kloot> I'm not sure I like two drawer classes for _every_ projectile subtype being added
<hoijui> no big hurry as long as nobody else wants to change projectiles a lot
<hoijui> most do not have a minimap drawer class
<hoijui> there is a common one that most use
<Kloot> that's true but all projectile types can use a common minimap and a common world drawer (although the code in them would become uglier)
<hoijui> i think it is 2 minimap drawers for the ~10 projectiles i already did
<jK> meh can't find it, I give it up for today
<hoijui> not sure what exactly yo umean kloot, you mean one class with two methods?
<Kloot> no, I mean this: (sec)
<hoijui> ahh i think i know what you mean
<hoijui> that would mean a lot of "engineering" though, right?
<zerver> i dont like the null checks in GetMinimapDrawer() and GetDrawer()
<hoijui> ah.. or a big switch
<zerver> you should always create the drawers, and remove the null checks
[22:52:00] Error: Command (// Note: This is never deleted, but it is to be (re-)moved anyway) does not exist, use /help for a list of available commands.
<hoijui> // Note: This is never deleted, but it is to be (re-)moved anyway
<Kloot> hoijui: http://pastebin.com/nXBjKMVZ
<hoijui> both these methods will be removed anyway zerver
<zerver> ok
<hoijui> just need to be there for the transition period\
<hoijui> mm ok kloot
<Kloot> so indeed a big switch
<hoijui> well... i do not care much
<hoijui> jk.. could that be the way they did it?
<Tobi> do put the code for each case in a separate method though, please :)
<hoijui> i think the way i did it is nicer, purely OO design wise, but if you want to keep new files lower, that is better
<hoijui> Kloot, is that your main concern, too many classes/files?
<Kloot> yeah your design is more OO-ish, but I dislike so many special-purpose files
<Tobi> since it's C++ we could stuff a bunch of classes in a single file
<hoijui> :D
<Kloot> files and classes*
<hoijui>
<Tobi> ah :)
<jK> yeah such a switch is a way to solve it
<hoijui> my aproach would allow the smae btw.
<hoijui> or say.. would allow it to mix
<hoijui> similar projectiles coudl share a drawer, using a switch internally (eg, all the laser ones)
<hoijui> and others coudl have separate ones
<hoijui> hmm.. woudl not make it much nicer i guess :D
<Kloot> heh no, there are only two laser types and all the others differ
<Kloot> three lasers*
<hoijui> yeah .. also.. it would possibly be harder to find the drawer that fits a (sim-)projectile
<zerver> yeah, u need a cabinet if the projectile is too big
<Tobi> single drawer class may be nice for stuff that can be shared among projectiles
<hoijui> that would be done with inheritance
<Tobi> yeah but that could get a bit messy in practice, I think (hard to follow the flow without a good IDE, as it might jump from class to class)
<hoijui> yeah that is true
<hoijui> if you use a good IDE, well defined methods, well documented...
<hoijui> i mean...
<hoijui> the idea of making methods is, to not have to know what the method does exactly
<hoijui> method name (plus possibly docu) shoudl be enough
<hoijui> my model also allows to change a renderer at runtime
<hoijui> easily/nicely
<zerver> one other think i dislike is the slight increase in dereferenced pointers "->"
<zerver> -think +thing
<hoijui> it also allows for easier profiling and the like
<Tobi> yeah true from OO POV your solution is nicest
<jK> k found it:
<hoijui> zerver, you mean the getters?
<jK> http://webcache.googleusercontent.com/s ... en&ct=clnk
<jK> http://webcache.googleusercontent.com/s ... en&ct=clnk
<zerver> yeah, u need to read member variables from the drawer classes
<jK> only available via google cache (not copied to new gamedev yet)
<Tobi> getters would be inlined in release build anyway I presume?
<jK> yup, as long as they aren't virtual
<zerver> from a performance pov, no difference, "this" pointer or some other pointer, i just mean for code readability
<hoijui> exactly
<jK> but not all derefernces are optimized, so having "->" in a for-loop is not always auto-optimized
<hoijui> but these getters surely all get inlined.. always
<hoijui> i is like.. the most simple optimization that would be done
<hoijui> could*
<Tobi> nothing of this matters anyway unless its _measured_ to be a bottleneck (sorry I'm so harsh on this but I really really hate premature optimization
)<hoijui> mmm
<jK> it's bad to continue bad designs
<hoijui> ?
<zerver> no, im just saying that "variable" is more readable than "p->variable"
<jK> when you know that something is slow, then you shouldn't continue to write it like that
<hoijui> thing is, if the virtual functions would proove to be bad, it could be changed to a compile time polymorphism still
<jK> we know that rendering is slow atm
<hoijui> also.. as the first article mentions, compilers do optmize
<jK> and we even know that minimap particle rendering is slow
<jK> and minimap particle rendering is pure c++ polymorphism
<Tobi> jK: unless the code is more readable with the slower code, or easier to get correct, or cleaner designed (re when you know that something is slow, then you shouldn't continue to write it like that)
<hoijui> so if the class you call the virtual function on has no classes that derive from it, it can be converted to a non-virtual cast
<hoijui> which would be the case in .. i think all the cases so far
<hoijui> ah . not for minimap maybe, but it coudl be done there too
<hoijui> having a defaultMinimapDrawer instead of using the parent one directly
<zerver> typically, if you write a function and there is a hundred "proj->" in it, then this means the function should be a member of the class in question
<hoijui> aehh.. no
<hoijui> in the engine we want sim and rendering splitted
<hoijui> drawing of a thing is not an integral part to it
<hoijui> therefore everything rendering related can not be a part of it/in it
<hoijui> guys.. you seem kind of .. absent
<hoijui> all doing other stuff too?
<hoijui> ahh nm, forget that
<zerver> this data duplication we spoke about earlier, do we really need it
<hoijui> can't remember
[23:20:32] Error: Command (/don't know what you mean) does not exist, use /help for a list of available commands.
<zerver> i mean, if you move rendering related variables into the rendering class, there will be less "proj->"
<jK> yeah, else multi-threading will always be buggy
<hoijui> ah yeah zerver.. that is true
<hoijui> the thing is... i see it this way:
<hoijui> a projectile consists of synced, unsycned and renderign related members
<hoijui> rendering related is really only what .. basically is opengl specific stuff
<hoijui> or say..
<jK> ? I thought hoij wanted to just move the functions to a diff class and reuse the data from the synced classes (when possible)
<zerver> i think strictly speaking we do not need this split, we could just make object deletion unsynced
<hoijui> if you want to have .. say... a directX renderer in the future, you would need all the unsycned stuff of a projectile too, like color
<hoijui> yeah jk, thats what i want
<hoijui> zerver, we want rendering code out of Sim
<hoijui> you cant do that without .. moving rendering code out of sim (hence, splitting it off from these classes)
<hoijui> splitting*
<hoijui> about special purpose classes: we alreayd have different classes for different projectiles, so i cant see why having the mfor drawing these projectiles is bad. it also allwos for stuff like:
<hoijui> having a kind of bundle of projectile class plus projectile rendering class as a compile-time-plugin
<hoijui> or say... having projectile types .. optional.. or however you want to put it
<hoijui> that means you could easily do special build of the engine, with.. for example only one projectile type
<Kloot> what would be the hypothetical purpose of that?
<hoijui> could be useful for testing or sync debugging or even for builds of the engine for low end systems, maybe
<hoijui> like.. having a mod that has only laser projectiles, running on a special build of the engine with a simplified rendering env
to be continued ...
(max message length reached, see next post)
