Torque 3D's Achilles heel
by Konrad Kiss · in Torque 3D Professional · 03/24/2012 (11:49 am) · 184 replies
Recently I spent some time getting myself familiar with static code analysis and the more common (indie-budget freindly) tools associated with the topic. #AltDevBlogADay had several posts about such tools by Bruce Dawson and John Carmack. Definitely worth checking out.
So, it all began with me having a very elusive bug somewhere in code. I hoped that it would be something that a static code analyzer would pick up, so I lined up what I could from trial versions to open source software, and got some amazing results once these went through the Torque 3D 1.2 codebase.
I can't remember if the actual problem was found by these tools (it's been a while ago). What I recall is being taken aback by the results I got. But first a disclaimer...
I've been reviewing engine code changes for the past 4 years. Torque 3D is huge. One of the most surprising thing was the fact, that these code analyzers could - proportionally - find only a few problems. However, the few they did find were hard to see even when I knew the problem.
The kinds of problems I found primarily were:
- Uninitialized member variables used without initialization
- Checking the existence of an object and using the same object outside the check condition
- Passing local pointers that could become invalid
- Using && instead of &, || instead of | operations
- Bad casts for variables while the code assumes another cast (ie. U32 vs S32 - especially when negative values are used for error codes - common in some of the linked libraries)
- Typos, cut & paste programming mistakes that compile (probably the most vicious of all)
I'm only going to list my favorites, hoping that this will be a big enough motivation for everyone to start using some basic form of static code analysis. The complete list would be way too long for me to list here.
gfxD3D9TextureManager: using boolean evaluation against variables that were clearly intended to be bitmasks.
guiIconButtonCtrl.cpp: find the highlighted button, I dare you
processedFFMaterial.cpp: making sure (though what about blendDest?)
mRectF.cpp: intersectTriangle
winDInputDevice.cpp: POV_down, not much of a choice (twice in the same file)
rectClipper: clipRect (near eof)
win32Window.cpp: equilibrium
Finally - I need to clarify - by all means, no disrespect was intended towards Torque 3D developers. I would have done far worse most likely. I can't count the many new things in the source that made me go "Wow" and "Yey" - it is clearly visible that the team is bigger and better at what they do. Torque 1.2 is the best Torque yet - and I mean it by comparing the overall usability, the number of bugs, etc..
With this post I'm only trying to point at a solution that can save everyone from such blatant bugs in the future, while - perhaps - also nudging GG a little to take a step in the right direction. ;)
Cheers
--Konrad
So, it all began with me having a very elusive bug somewhere in code. I hoped that it would be something that a static code analyzer would pick up, so I lined up what I could from trial versions to open source software, and got some amazing results once these went through the Torque 3D 1.2 codebase.
I can't remember if the actual problem was found by these tools (it's been a while ago). What I recall is being taken aback by the results I got. But first a disclaimer...
I've been reviewing engine code changes for the past 4 years. Torque 3D is huge. One of the most surprising thing was the fact, that these code analyzers could - proportionally - find only a few problems. However, the few they did find were hard to see even when I knew the problem.
The kinds of problems I found primarily were:
- Uninitialized member variables used without initialization
- Checking the existence of an object and using the same object outside the check condition
- Passing local pointers that could become invalid
- Using && instead of &, || instead of | operations
- Bad casts for variables while the code assumes another cast (ie. U32 vs S32 - especially when negative values are used for error codes - common in some of the linked libraries)
- Typos, cut & paste programming mistakes that compile (probably the most vicious of all)
I'm only going to list my favorites, hoping that this will be a big enough motivation for everyone to start using some basic form of static code analysis. The complete list would be way too long for me to list here.
gfxD3D9TextureManager: using boolean evaluation against variables that were clearly intended to be bitmasks.
if (retTex->mProfile->isRenderTarget() && mslevel != 0 && (mDeviceCaps.Caps2 && D3DDEVCAPS2_CAN_STRETCHRECT_FROM_TEXTURES))
guiIconButtonCtrl.cpp: find the highlighted button, I dare you
ColorI fontColor = mActive ? (highlight ? mProfile->mFontColor : mProfile->mFontColor) : mProfile->mFontColorNA;
processedFFMaterial.cpp: making sure (though what about blendDest?)
result.blendSrc = GFXBlendOne;
result.blendSrc = GFXBlendOne;mRectF.cpp: intersectTriangle
if(contains(a) || contains(b) || contains(b))
winDInputDevice.cpp: POV_down, not much of a choice (twice in the same file)
newEvent.objInst = ( objInst == 0 ) ? SI_DPOV : SI_DPOV
rectClipper: clipRect (near eof)
out_rRect.extent.x = bottomR.x - out_rRect.point.x + 1; out_rRect.extent.x = bottomR.y - out_rRect.point.y + 1;
win32Window.cpp: equilibrium
newLeft = mClamp(newLeft, 0, newLeft);
newTop = mClamp(newLeft, 0, newTop);Finally - I need to clarify - by all means, no disrespect was intended towards Torque 3D developers. I would have done far worse most likely. I can't count the many new things in the source that made me go "Wow" and "Yey" - it is clearly visible that the team is bigger and better at what they do. Torque 1.2 is the best Torque yet - and I mean it by comparing the overall usability, the number of bugs, etc..
With this post I'm only trying to point at a solution that can save everyone from such blatant bugs in the future, while - perhaps - also nudging GG a little to take a step in the right direction. ;)
Cheers
--Konrad
About the author
http://about.me/konrad.kiss
#62
05/13/2012 (9:15 am)
Sorry bout the spelling typing on phone
#63
If anything weird happens, we can always ask for help from each other, no big deal.
05/13/2012 (9:20 am)
Yeah, but only one person writing wouldn't be too effective. I believe there will rarely be two people working at the same time, and even then, merge conflicts won't be common as long as we all frequently update and commit, but here's git's take on handling merge conflicts.If anything weird happens, we can always ask for help from each other, no big deal.
#64
05/13/2012 (9:30 am)
Aren't a merge conflict issue, but are a problem of code conflict.
#65
05/13/2012 (9:34 am)
I think we can solve such cases together by discussing the code via Github's code review feature. We'll have to try and see how that works out and adapt if it doesn't.
#66
05/13/2012 (9:37 am)
Another option is that we just split the engine into sections and each dev is responsible for there part. If our goal is to just fix the u32 s32 issue first then i dont see a reason why not
#67
05/13/2012 (9:37 am)
For what concerns my current job of optimizing:- I have corrected more 60 of the coding errors (errors not seen as serious errors, so aren't bugs). Anyone who would write the code im that way.
- I replaced the memory and string functions, with versions written in asm and higher performance (cross platform). There are many calls to memcpy or memset, so why not try to save some cpu cycles?
- Replaced the bullet with the new version, which in part runs on the GPU due to the use of OpenCL. New versions of the bullet, now allow take advantage of the gpu. So i think that many will soon abandon PhysX (closely to the nvidia card only). APEX? In the current bullet SDK is already available a code sample of the Random Voronoi Fracture on non-precomputed mesh.
- After some phase of profiling, i have lightened some parts of the engine (such as the flush of some D3D querys). I don't have to mount all the shelves, because it is written in the manual of my shoe rack purchased in ikea. Are only guidelines ;)
#68
My quick and dirty first round agenda would be:
- ~90 bugs / typos found and fixed using CPPCHECK and PVS Studio (static code analyzers) perhaps we should keep going through the code using new analyzers as each new analyzer finds something.
- A more optimized and up-to-date implementation of my anonymous function declarations resource (TorqueScript).
- Sahara - it's a shader feature. ( example ) Stuff like this is becoming standard in other engines.
- Changes to be able to run a dedicated server and any number of clients locally (ie. much easier debugging) with separate log files.
- Changes to run the game as a non-administrator by writing into the Windows user's AppData directories instead of the game directory.
- Some in-house tools
05/13/2012 (10:04 am)
Wow! We need to get this started asap! :)My quick and dirty first round agenda would be:
- ~90 bugs / typos found and fixed using CPPCHECK and PVS Studio (static code analyzers) perhaps we should keep going through the code using new analyzers as each new analyzer finds something.
- A more optimized and up-to-date implementation of my anonymous function declarations resource (TorqueScript).
- Sahara - it's a shader feature. ( example ) Stuff like this is becoming standard in other engines.
- Changes to be able to run a dedicated server and any number of clients locally (ie. much easier debugging) with separate log files.
- Changes to run the game as a non-administrator by writing into the Windows user's AppData directories instead of the game directory.
- Some in-house tools
#69
Phew! Glad I've finally got that off of my chest! :P
05/13/2012 (11:55 am)
Incidentally Konrad, and totally nitpicking here ... the music on your Sahara video is quoted as being by Jonathon Greer, when it's actually CONSTELLATIONS (by Chill Purpose)Phew! Glad I've finally got that off of my chest! :P
#70
05/13/2012 (12:10 pm)
Ouch, thanks Steve! It looks like I referred the wrong "Constellations". There are actually more it seems. I fixed it with an annotation. Thanks for saving me from 30 to life! :) 
#71
Is there a possibility of a potential "split" between a Torque 3D CEV and an official Torque 3D codebase? Yes. But this can be resolved by keeping GG apprised of the fixes and improvements that we can do while waiting for them to cycle back and forth between their engines. This could take the form of weekly, or allowing sufficient time for QA maybe monthly difs/patches to be submitted to them -- they seem to move at a slower pace than we want ;)
Would I want to branch and introduce totally new features at some point? Yes. You could think of it as a R&D branch that all who participate in could benefit from at their discretion.
We should still have discussion and even voting towards what large-scale changes are introduced.
With a good design in commit philosophy merge conflicts shouldn't be that much of a worry. GitHub seems to have the tools necessary to resolve any such conflict that may arise, as should any good version control client to be used at our end. With as many collaborators as this could have potential for we should stress making small and incremental "atomic" commits instead of one large commit of many unrelated changes. Files can be locked and should be allowed, but locking entire directories should be discussed beforehand. Good communication is actually the best defense against such conflicts.
05/13/2012 (12:13 pm)
To clarify the 'community enhanced version' words that I tossed out there. Bug fixes were the primary goal of the TorqueX CEV. Bugs found with the software that certain end users in the community felt should be fixed. Issues that just didn't meet the priorities of GarageGames at the time. I see Torque 3D as being in the same position. All (or many) of these little (some not so little) issues that Konrad, Alfio, and others have and are pointing out are things that have been mentioned before, yet never resolved. The bad typecasts, the typos, the wtf! logic, etc, all of these things may be known but have yet to meet a priority level sufficient enough to be officially fixed.Is there a possibility of a potential "split" between a Torque 3D CEV and an official Torque 3D codebase? Yes. But this can be resolved by keeping GG apprised of the fixes and improvements that we can do while waiting for them to cycle back and forth between their engines. This could take the form of weekly, or allowing sufficient time for QA maybe monthly difs/patches to be submitted to them -- they seem to move at a slower pace than we want ;)
Would I want to branch and introduce totally new features at some point? Yes. You could think of it as a R&D branch that all who participate in could benefit from at their discretion.
We should still have discussion and even voting towards what large-scale changes are introduced.
With a good design in commit philosophy merge conflicts shouldn't be that much of a worry. GitHub seems to have the tools necessary to resolve any such conflict that may arise, as should any good version control client to be used at our end. With as many collaborators as this could have potential for we should stress making small and incremental "atomic" commits instead of one large commit of many unrelated changes. Files can be locked and should be allowed, but locking entire directories should be discussed beforehand. Good communication is actually the best defense against such conflicts.
#72
Would there be a way to upload resources as additions to the codebase, but not part of the head? I would like a good way to share resources with the community and I was planning on creating a repository. I would love to have my resources be part of a main community repository. The one resource I am thinking of right now is my scripting language extension which I have not released yet, but am getting close.
I know the initial focus will be on bug fixes, but as Konrad mentioned new features would be desired too.
I would vote for Xp-Dev. You pay $5 a month and it gives you 2GB space. If we are only hosting source files that should be plenty. I just checked and it is 20MB space for just the engine source.
05/13/2012 (10:59 pm)
Michael,Would there be a way to upload resources as additions to the codebase, but not part of the head? I would like a good way to share resources with the community and I was planning on creating a repository. I would love to have my resources be part of a main community repository. The one resource I am thinking of right now is my scripting language extension which I have not released yet, but am getting close.
I know the initial focus will be on bug fixes, but as Konrad mentioned new features would be desired too.
I would vote for Xp-Dev. You pay $5 a month and it gives you 2GB space. If we are only hosting source files that should be plenty. I just checked and it is 20MB space for just the engine source.
#73
Potential pitfall: would access be restricted to 1.2 owners, and henceforth for only owners of the most recent update? More broadly, would fixes be considered for older engine versions at all?
The whole idea takes me back to the good old days when there was a GG-hosted CVS repository that licensees grabbed code from. (Yes, that was before I even knew what Torque was. But read enough old forum posts and begin to you feel like you were there yourself...)
05/14/2012 (3:01 am)
This is a pretty cool idea. To start with, I was a little miffed at what would involve basically a branch of T3D where we do GG's job, but I'm warming to the idea. Not sure if a monthly fee is worth it for me at this point, but it's something I'd definitely consider once things were up and running.Potential pitfall: would access be restricted to 1.2 owners, and henceforth for only owners of the most recent update? More broadly, would fixes be considered for older engine versions at all?
The whole idea takes me back to the good old days when there was a GG-hosted CVS repository that licensees grabbed code from. (Yes, that was before I even knew what Torque was. But read enough old forum posts and begin to you feel like you were there yourself...)
#74
Daniel does raise a good question about what version of the engine would be supported. I don't think that trying to support all versions of T3D would be feasible. At most maybe a 1.1 patched to 1.2 branch, and a full 1.2 branch. This kind of setup should make it somewhat easy to merge differences between the two, but we would still need to limit access to the full 1.2 branch. Or we could just go all out and say 1.2 ONLY excluding those, like me, who haven't upgraded -- that's what discussion is for.
I don't think there is any way of avoiding the fee. Even if someone took on paying for the repository on their own and provided access to all licensees, just how long would that last? A month, 6 months, a year, more? That is a key point in where other community collaborative projects have fallen apart. Many never even get started because they are wanting GarageGames to officially host and provide the bandwidth/storage.... I just don't see this happening. And with Konrad's idea of using the surplus donation to hire programmers to include new features for our (everyones) benefit, that's just icing on the cake :)
05/14/2012 (8:43 am)
@Frank: you could always share a diff/patch file against either stock code or from the theoretical T3Dcev.Daniel does raise a good question about what version of the engine would be supported. I don't think that trying to support all versions of T3D would be feasible. At most maybe a 1.1 patched to 1.2 branch, and a full 1.2 branch. This kind of setup should make it somewhat easy to merge differences between the two, but we would still need to limit access to the full 1.2 branch. Or we could just go all out and say 1.2 ONLY excluding those, like me, who haven't upgraded -- that's what discussion is for.
I don't think there is any way of avoiding the fee. Even if someone took on paying for the repository on their own and provided access to all licensees, just how long would that last? A month, 6 months, a year, more? That is a key point in where other community collaborative projects have fallen apart. Many never even get started because they are wanting GarageGames to officially host and provide the bandwidth/storage.... I just don't see this happening. And with Konrad's idea of using the surplus donation to hire programmers to include new features for our (everyones) benefit, that's just icing on the cake :)
#75
Another place we could consider hosting is TFS on Azure, I have been actively using Microsoft's preview server and have had little or now downtime, and its pretty easy to manage and it is secure. There is also a good chance we could put up a build server to validate checkins. I wouldn't mind helping with the administration of a TFS instance of the codebase. Maybe GG could put a small portal up that would allow an administer of this (and TorqueX cev) to check for licensing? It could be as simple as enter an email address to see which products the person is licensed for?
I agree with some of the other comments, but I think that one of the big issues TorqueX cev seemed to experience was that there was a lot of initial excitment, and then it fizzled because of individuals priorities and availability changed. I think that a monthly (althought it should be paid annually) fee is reasonable too, for those of us that are serious developers, it is well worth it. That would also help insure the longevity of the resource.
I also think that we should try to figure out as part of this how to host some of the static files (lots and lots of links are currently broken in the forums and on TDN), it is frustrating for people new to the engine to see broken links when they find something of interest.
We need to consider how do we merge everything together in T3D 1.4 comes out? Handling of licening then as well, assuming it could be a paid upgrade, it would require more management around permissioning. Will GG take our fixes and incorporate them, if so, will there be a process that we use to collaborate with them to help make sure it happens?
If we are going to do this, we should use the same process and community to take care of the other engines, that way everything is consolidated, and more likely to succeed. I have licenses to pretty much all of the products; I'd like to see a single more unified community around fixes, and the like.
There are lots of little details that I think we should understand as far as how this works now, near-term and longer term.... Whatever we do the important thing to me is that it is managed and sustainable.
05/14/2012 (9:41 am)
We could also create one or more branches that could include merges of the other more common addons (AFX for example). It seems like all this could be easily managed with permissions within TFS. I am not sure how granular SVN is with its permissioning; I know that I have restricted access to branches and even portions of branches within TFS.Another place we could consider hosting is TFS on Azure, I have been actively using Microsoft's preview server and have had little or now downtime, and its pretty easy to manage and it is secure. There is also a good chance we could put up a build server to validate checkins. I wouldn't mind helping with the administration of a TFS instance of the codebase. Maybe GG could put a small portal up that would allow an administer of this (and TorqueX cev) to check for licensing? It could be as simple as enter an email address to see which products the person is licensed for?
I agree with some of the other comments, but I think that one of the big issues TorqueX cev seemed to experience was that there was a lot of initial excitment, and then it fizzled because of individuals priorities and availability changed. I think that a monthly (althought it should be paid annually) fee is reasonable too, for those of us that are serious developers, it is well worth it. That would also help insure the longevity of the resource.
I also think that we should try to figure out as part of this how to host some of the static files (lots and lots of links are currently broken in the forums and on TDN), it is frustrating for people new to the engine to see broken links when they find something of interest.
We need to consider how do we merge everything together in T3D 1.4 comes out? Handling of licening then as well, assuming it could be a paid upgrade, it would require more management around permissioning. Will GG take our fixes and incorporate them, if so, will there be a process that we use to collaborate with them to help make sure it happens?
If we are going to do this, we should use the same process and community to take care of the other engines, that way everything is consolidated, and more likely to succeed. I have licenses to pretty much all of the products; I'd like to see a single more unified community around fixes, and the like.
There are lots of little details that I think we should understand as far as how this works now, near-term and longer term.... Whatever we do the important thing to me is that it is managed and sustainable.
#76
The main downside is that you wouldn't have multiple concrete users when looking at commits from the write account, but with DCVS that's less of an issue since you can commit locally.
Also, BitBucket allows users to checkout a Git or Hg repo via SVN, so that would be helpful for those that don't want to switch away from SVN or learn a new versioning tool just for this project.
I honestly think that it'd be the simplest method for this and could allow everyone to sidestep the complexities involved with maintaining a paid GitHub account entirely.
Edit: A combination of the 5 accounts could be used to separate access between 1.1 (patched to 1.2) and full 1.2 as well. One read and write account for each, and maybe a "free for all" account that works off of a separate branch or something. Or, alternatively, it could just be two separate BitBucket repos (one for 1.1 and one for 1.2) which have 3 accounts each (1 read, 1 write and 1 for the branch of that repo which anyone can read or write to).
05/14/2012 (11:42 am)
I haven't read all the previous replies yet, but one easy (and free) way we could do a private repo is to host it on BitBucket. It allows for free private repos with unlimited size, with a limit of 5 users. We could set up two main accounts and just share them: one for read access and one for write access.The main downside is that you wouldn't have multiple concrete users when looking at commits from the write account, but with DCVS that's less of an issue since you can commit locally.
Also, BitBucket allows users to checkout a Git or Hg repo via SVN, so that would be helpful for those that don't want to switch away from SVN or learn a new versioning tool just for this project.
I honestly think that it'd be the simplest method for this and could allow everyone to sidestep the complexities involved with maintaining a paid GitHub account entirely.
Edit: A combination of the 5 accounts could be used to separate access between 1.1 (patched to 1.2) and full 1.2 as well. One read and write account for each, and maybe a "free for all" account that works off of a separate branch or something. Or, alternatively, it could just be two separate BitBucket repos (one for 1.1 and one for 1.2) which have 3 accounts each (1 read, 1 write and 1 for the branch of that repo which anyone can read or write to).
#77
05/14/2012 (11:52 am)
Interesting suggestion by Ross. It would require some coordination and communication before using the write access accounts, but that kind of setup could definitely get things started.
#78
But then how do we handle verifying accounts? If we share one access to many users and there's no entrance fee, what will stop anyone from spreading access credentials like wildfire?
I'm worried it would make pirating the engine even more convenient. Anonymity wouldn't do good imho.
05/14/2012 (12:06 pm)
I'm a little worried about sharing access like that, but it would definitely be easier to get started. I don't mind not having to take on responsibilities either to be honest.But then how do we handle verifying accounts? If we share one access to many users and there's no entrance fee, what will stop anyone from spreading access credentials like wildfire?
I'm worried it would make pirating the engine even more convenient. Anonymity wouldn't do good imho.
#79
05/14/2012 (12:12 pm)
Good point Konrad. I didn't even think about that... although one bad user on the per licensed account access could still cause the same damage.
#80
This could be done by creating a branch for each user if they want write access until they become "trusted" by the community, and then they could get write access to the main dev branch.
Others do this by submitting patches to one of the writers. If you want to go that route, I wouldn't mind helping with applying and verifying the patches for those who are not "trusted" yet.
The other thing we could do is allow users to ask what type of access they want, Read or Read-Write, It would be relatively easy to setup a build process that would manage the automatic creation of source archives that are prebuilt either per check in, or nightly.
05/14/2012 (12:20 pm)
It needs to be one user per account. (There's not limit, other than size currently to the number of users on the TFS Azure site).This could be done by creating a branch for each user if they want write access until they become "trusted" by the community, and then they could get write access to the main dev branch.
Others do this by submitting patches to one of the writers. If you want to go that route, I wouldn't mind helping with applying and verifying the patches for those who are not "trusted" yet.
The other thing we could do is allow users to ask what type of access they want, Read or Read-Write, It would be relatively easy to setup a build process that would manage the automatic creation of source archives that are prebuilt either per check in, or nightly.
Torque Owner Vince Gee
WinterLeaf Entertainment