Present: zerver, Kloot, hoijui, abma, jK
agenda
- include http://code.google.com/p/bitwise-enum/ ?
conclusion: waiting for authors response - SIGFPE with DEBUG build at start
conclusion: LuaParser.cpp isn't NaN-safe yet - //! ?
conclusion: please don't use this for comments, this will make doxygen generated documentation unuseable - please use lobby.springrts.com as main-lobby server address!
conclusion: please use it, taspringmaster.clan-sy.com will be deprecated - Release plan
conclusion: few major bugs left, looks like we have to make a test-release soon
Welcome
<abma>hey!
<zerver>hi
<hoijui>hey!
<abma>afaik tvo is on vacation
include http://code.google.com/p/bitwise-enum/ ?
<hoijui>should this be in minutes at all? i forgot what the decission was last time...
<hoijui>jk?
<abma>afaik the license is incompatible
<abma>its gpl3 and that doesn't work with gpl2 (?)
<hoijui>mmm
<hoijui>ahh yeha right, jk wanted to talk to the author
<jK>I contact him and he is checking FSF
<jK>+ed
<hoijui>ook
<hoijui>thanks
SIGFPE with DEBUG build at start
<hoijui>dont know if this shoudl be in minutes either....
<hoijui>with DEBUG build, you get no SIGFPE in BA or ZK, jk?
<hoijui>..or anyone>?
<abma>the question is, can this sigfpe's fixed?
<jK>no reason to have it not in minutes many ppl just like it to have regulary something to read
<abma>as it seems that -fsignaling-nans to be enabled for the complete binary
<abma>(or i/we made something wrong)
<zerver>it is for the whole binary
<jK>with my default dev enviroment (still based on CA) I don't get any SIGFPE atm
<hoijui>:D
<jK>but as abma mantis ticket showed LuaParser.cpp isn't NAN safe yet
<hoijui>based on CA?
<zerver>no SIGFPE with MSVC either, because I did not enable it :)
<jK>and I don't know how to treat NANs in there correctly
<abma>some lua-widget produces this SIGFPE, this is why i added the stacktrace for lua, too
<hoijui>for the signaling-nans change (defualt on wiht DEBUG build) to take effect, you have to either clear hte cache and reconfigure (CMake), or set it manually on configure
<jK>the best way would be to return an error message with the location of the NAN (UnitDef/WeaponDef/... + tag)
<abma>afaik without that its really hard to find the script, that causes that SIGFPE
<hoijui>for me, the SIGFPE already happens in LoadScreen
<hoijui>i guess it cant be a widget then
<jK>it's LuaParser.cpp ...
<jK>other Lua's are safe now
<hoijui>ah yeha right
<abma>ok... so all SIGFPE's can be catched and this makes next point obsolete?
<abma>or would this lua-trace still make sense?
<abma>i mean next point in the agenda
<jK>stacktrace wouldn't catch issues in LuaParser (the NANs occur in the c-interface, which makes error reporting such complicated)
<jK>yeah next
//! ?
<jK>tvo isn't here
<abma>didn't know that it was his topic
<abma>i guess he will complain about that tag, because its a doxygen tag
<hoijui>tvo, jk and me discussed it
<jK>k anyone else got an opinion on that?
<abma>hm,using these tags makes the doxygen docu a bit ...ehm confusing
<hoijui>me (and i would say tvo too) thinks its a hack
<hoijui>it is abusing the tag, and garbles the documentation
<hoijui>coments end up where they should not be
<abma>sorry, can't show examples in current doxygen, my vserver is down :-/ (to cheap hoster)
<zerver>:-\
<hoijui>the pro argument, if i got that right, is that the comments are colored differently then out-commented code with this
<hoijui>for most IDEs
<hoijui>in*
<jK>I still like to have a style to differ between comment and deactivated code
<jK>yup
<abma>is there maybe a no-doxygen tag for that?
<abma>or a //!<exclude from doxygen> </exclude>?
<hoijui>springs policy has genreally been, that code should not be outcommented, but removed
<hoijui>and this is mostly the case
<hoijui>(makes sense as the code remians in history anyway)
<hoijui>there is not muhc outcommented code
<jK>sometimes you want to keep deactivated code
<jK>e.g. you replace a very simple code line with an uber-optimized 10lines code that is much faster than the simple & easy to read one. then you want to keep the simple code to `explain` what the uber-complexe one is doing
<hoijui>well, then you have to live wiht that beeing colored the smae way as comments
<hoijui>or you add a custom macro
<abma>hm, then it would make sense to make a real doxygen-comment?
<hoijui>to color code differently, using eg the rule i suggested
<hoijui>if there is a space after the //. then its a comment, otherwise code
<abma>only bad with //! is, that it simple appends this text to the function description/text
<hoijui> //! is an API comment
<hoijui>and jk uses it for commenting non API stuff
<hoijui>so its a hack
<hoijui>its not somethign missing in doxygen
<hoijui>it is*
<jK>doxygen fails :<
<hoijui>nope
<hoijui>you are misuing a standard
<hoijui>misusing*
<hoijui>well. shoudl we vote on that?
<abma>hm.. is //# a doxygen tag?
<abma>or //+?
<jK>meh, no need, it will be hard but I try to stop using it then :'(
<abma>it can be used, but then please write doc-strings :)
<abma>hm... then next?
<hoijui>thanks
please use lobby.springrts.com as main-lobby server address!
<hoijui>:D
<hoijui>yeah.. i guess that is here so many people will read it?
<abma>yeah, nothing to say here if there are no questions
<hoijui>
<abma>then...
Release plan
<abma>i didn't test much, but most bugs in mantis seems to be fixed
<zerver>yep, we can't wait forever with this release
<hoijui>ah yeah.. coudl do test release
<jK> http://springrts.com/mantis/view.php?id=2511
<jK>-> blocker
<jK>+ UnsyncedHeightMap still got some bugs (working on them & have a few question at kloot)
<jK>+ Kloot's new Lua functions need to get improved too
<jK>+ https://github.com/spring/spring/commit ... 8#comments is still a bug into my mind
<jK>-> put a assert in front of the if-clause -> crash directly at the start of the game
<zerver>unitscript still crashes?
<Kloot>say what needs to be improved then
<jK>no your `fix`, breaks code
<jK>LuaUnsyncedCtrl::SetMapSquareTexture
<Kloot>I gathered that
<Kloot>what needs to be improved _about it_
<Kloot>?
<jK>first it's Get & Set are in the same Lua....cpp
<jK>GetMapSquareTexture should be moved into LuaOpenGL.cpp
<Kloot>imo luaopengl should be purely for wrappen opengl api funcs, but ok
<jK>& writing into LuaTextures & NamedTextured(!!!) is bad very bad
<jK>it should be moved into a new magic name in gl.Texture()
<zerver>jK, if unitscript is broken, explain it in mantis. I just tried to fix your code since it crashed.
<Kloot>nothing is written into either of those
<jK> https://github.com/spring/spring/blob/5 ... .cpp#L1565
<jK>you override LuaTexture & NamedTexture data in there
<Kloot>sigh
<Kloot>no
<Kloot>I _read_ from them to bind a map-square texture
<Kloot>hence the const's
<jK>oh nvm
<jK>still you override a LuaTexture in GetMapSquareTexture
<Kloot>well yes, that's what CreateTexture is supposed to be used for
<jK>the problem is that the lua interface is limited for texture handling, and LuaTextures are meant for rendering into them
<jK>but you can't render into compressed textures
<Kloot>I know, but that's not what happens
<Kloot>the dxt data is just copied, later you can rtt it to a regular fbo'ed texture
<Kloot>copied and extracted*
<jK>extracted?
<jK>@zerver, it didn't crashed for me and you never gave a reproducable example
<zerver>but you are not telling how it breaks lua code either...
<hoijui>hint: duplicated Lua fucntion, metal map
<jK>I am not saying it breaks lua code
<zerver>ok?
<jK>I said it breaks animation scripts in general
<jK>COB+LUS
<jK>and as said put a assert in front of the if-clause and see it crashing at start
<zerver>assert what?
<jK>the if-clause (to see what the if-clause is `killing`)
<jK>before it didn't killed such requests, now it does -> changed behavior -> bug
<zerver>I think you are wrong there, please provide an actual example of what it breaks
<jK>as said it blocks code that worked before (as the assert proves)
<zerver>"blocks" is a rough term
<jK>that's what it does
<zerver>not really, if the script is finished, it will be removed anyway, just though a different mechanism (one that does not crash)
<jK>AddInstance() doesn't remove
<jK>-> don't argue the change in UnitScript.cpp, I only talk about the UnitScriptEngine.cpp change
<jK>+I
<zerver>Correct, AddInstance() doesn't remove, but in that case the script is already registered in the engine
<zerver>if the script is current, it has to be registered already...
<jK>and what if Tick() returns to erase the anim instance?
<zerver>you mean returns -1 ?
<zerver>oh wait that has changed it seems
<zerver>CUnitScriptEngine::Tick is responsible for erasing the current anim, yes
<zerver>what is the problem with that?
<jK>if the script wants to add a new anim, but wants to delete the current one
<jK>before it run AddInstance and added the instance a second time, and the old one was deleted
<jK>now it doesn't add it a second time, and still deletes the current/old one
<zerver>only if it is empty (has no animations)
<jK>best way is to use a std::map/hashmap as the comment in the code suggest
<zerver>yeah, just have to index the map by something that is sync safe
<jK>give each CUnitScript an id counter?
<zerver>yup
<jK>hmm after your explanations your code makes more sense, still either add comments or use hashmap
<jK>k kloot here?
<jK>I am working on UnsyncedHeightMap
<Kloot>y
<jK>UHM broke a lot of texture updates e.g. in BumpWater, fixed now via the new CEventClient event
<jK>e.g. when the square wasn't in view of the heightmap change and they entered into view later, the texture wasn't updated
<jK>there are still some of such problems left, but it's not I want to know from you
<jK>I want to know what you proposed with the inLos member of the struct HeightmapUpdate
<jK>and what the rawVertexNormals are for
<Kloot>that was used to remember if a square was in los when I added delayed updates of the UHM
<Kloot>and rawVertexNormals relate to visVertexNormals as heightMapSynced relates to heightMapUnsynced
<jK>rawVertexNormals is still unsynced
<Kloot>yep
<jK>and only used as a buffer before visVertexNormals
<Kloot>that's why I just named it "raw"
<jK>it isn't used else where, also if update.los is false it doesn't make sense to update the texture at all
<jK>because then visVertexNormals isn't updated
<Kloot>not just the texture has to be updated
<jK>but you still update the texture
<jK>with old data
<Kloot>the texture update is combined with updates of the normals etc
<Kloot>that is the reason
<jK>k so I will decouple that
<Kloot>if a square was in los at the time the heightmap changed, all of those elements receive an update
<Kloot>as is required
<jK>and rawVertexNormals seems to be redundant, too, cause it doesn't solve the problem with delayed updates (in los when event was given, and los lost when it get processed)
<Kloot>RVN isn't meant to solve that problem at all, it is just necessary to know the true state of the normals
<jK>it is not the true state
<jK>the true state is synced
<Kloot>so when a delayed update is processed the "unsynced" normals can be overwritten
<jK>?
<Kloot>the raw normals are updated on every terrain deformation regardless of los
<jK>sure, but they aren't read
<jK>so they have no usage atm
<Kloot>static float3* rvn = &rawVertexNormals[0]; static float3* vvn = &visVertexNormals[0];
<Kloot>vvn[vIdxTL] = rvn[vIdxTL];
<jK><[LCC]jK> and only used as a buffer before visVertexNormals
<jK>you could just make a temp var in there and use that, it would have the same usage
<Kloot>no, aargh
<Kloot>are you saying that instead of rvn[vIdxTL] = vn.ANormalize(); you want to have "float3 bla = vn.ANormalize" and use bla to update vvn?
<jK>yup
<Kloot>ok
<Kloot>that would work but ONLY because the new normals are calculated in the same place as where the texture gets updated
<Kloot>(which is done for efficiency)
<jK>why?
<jK>you mean the face normals?
<Kloot>new vertex normals
<jK>those in vnn?
<Kloot>no, the raw vn's
<jK>so I can remove rvn?
<jK>because when that is removed, the whole block skipped if update.los === false
<jK>+can be
<jK>or better the whole function
<jK>making the los member in the HeighmapUpdate struct redundant
<jK>-> can use SRectangle
<Kloot>I'm trying to remember if there was a reason UpdateHeightMapUnsynced is called in the first place for out-of-los blocks
<jK>you wanted to fix the problem: "in los when event was given, and los lost when it get processed"
<jK>but RVN won't fix that w/o linking it with synced code somehow
<jK>only solution I see for this issue is to _force_ an UpdateHeightMapUnsynced when in synced code a terrain gets changed that still is in the unsynced queue
<Kloot>mmmyeag
<Kloot>h*
<jK>would need to modify the CRectangleOptimizer for that case then ^^
<Kloot>the whole code is now completely unintuitive ><
<Kloot>after HeightMapUpdateFilter was introduced that is
<jK>it will get cleaner with CRectangleOptimizer + CEventClient
<jK>at best there will be a SyncedHeightMapUpdate CEventClient event, too
<Kloot>hmm, that could make mapdamage a lot cleaner
<jK>already removed 2 calls from that stack there already ^^
<jK>-2n already
<Kloot>hehe
<jK>additionally lua gets a new event (duno how much sense it makes in there, but new functionality \o/)
<Kloot>if you post the event before the actual deformation then gadgets can choose to block them
<Kloot>or alter the intensity (crater formation density/distribution)
<jK>hmm then it needs 2 events (one to block & one to update data when it got done)
<jK>gadgets could do the same when they reset the heightmap in the event
<Kloot>might indeed be overkill with all the heightmap functions already available
<hoijui>done?
<zerver>seems so
<hoijui>night!
<zerver>gn, dream about our next release
<abma>:D
<hoijui>:D
<Kloot>still no concrete release plan?
<abma>ehm
<hoijui>i really hope that not!
<abma>very soon!
<hoijui>:D
<abma>gn8! :)
<Kloot>I've been hearing very soon very long
<abma>me to :-/
<Kloot>now it's getting ridiculous ^^
<zerver>very soon means on friday
<abma>hmm, friday would be good for a test-release
<abma>btw. macos buildbot now creates a .app
<abma>(don't know if it works)
<Kloot>that also sounds like a good excuse to release _something_
<abma>better than nothing ^^
<Kloot>joking aside, it is just irresponsible wrt. the project
<Kloot>to let so much time pass between releases
<abma>yes, thats really bad :-/
<abma>for example assimp still is nearly untested
<Kloot>that, and the thousands of other changes
<abma>yep
<zerver>yeah, 1 year between releases is too long
<Kloot>even 6 months is pushing it, at least at our level of development
<abma> 2083 files changed, 259611 insertions(+), 104124 deletions(-)
<abma>(git diff --stat 0.82.7.1)
<Kloot>721 insertions per day
<Kloot>assuming 12*30 days
<abma>the current repo has ~3600 files / 700k lines or so
<abma>there are some replaces...
<Kloot>and let's say there is a bug in 5% of those
<abma>but imo this shows how much was touched
<abma>:D
<abma>hopefully no major one
<Kloot>~13000 issues waiting to be discovered
<abma>hm, lets see how many bugs are found after a release :D
<Kloot>I can't wait
<abma>hm, then i think you've to test...
<abma>why do i get: "Warning: [good_fpu_control_registers]..." at start?
<Kloot>was slightly sarcastic
<abma>afaik this happened only with an attached gdb
<abma>uh, doh!
<abma>forget it
<abma>its after the stacktrace (because of the SIGFPE...)
<jK>gn8
<Kloot>and gn from me
