search

FreeFalcon / freefalcon-central

A campaign based, multiplayer, open source flight simulator.
http://freefalcon.org/
BSD 2-Clause "Simplified" License
159 stars 99 forks source link

Strange indexing overflow crashing on startup #5

Closed vinnydiehl closed 11 years ago

vinnydiehl commented 11 years ago

Our next problem is here.

The problem is that ppn->sizeStages somehow jumps way up to some obscenely high number after a few of what appears to be normal iterations of that On block.

I put in a bit of test code to send the value of that to a file so I could observe its pattern, here are my findings: https://gist.github.com/53d3e98c47848414e4e6

The bit of code that I had there commented out does stop the crash, but I feel like that's a pretty bad thing to do? We should probably find out why the value of ppn->sizeStages suddenly jumps way up to 1064514355 and stop it from doing that.

PeterMeyer commented 11 years ago

Oh my... i hate all the ifdefs there.. you never know what side effects this causes. Will dig into it.

vinnydiehl commented 11 years ago

If you dig in and can't find the cause of the jump, look into whether or not my hacky fix (the one I commented out in the Gist) would come back to bite our asses down the road. :confounded:

Oh my... i hate all the ifdefs there.. you never know what side effects this causes.

Looks like a lot of switches for new features, in the future we'll look into getting rid of as many as we can.

PlutoniumHeart commented 11 years ago

It looks like when parsing the particlesys.ini file the exe doesn't know how do parse entries starting with the word "trail" so it keeps thinking that all the words parsed belong to the last entry scaned which is "id=wp_small", that's what's causing the over float.

Don't know how the trails section was originally parsed, if someone can give me some ideas, I can fix this.

I also can't help wondering, why this source code has some many bugs that we can't even get into the game? Was it because of the VS upgrade, or it is a older devlopment version? I would really like to know about these as well.

Thanks!

Plutonium

PlutoniumHeart commented 11 years ago

One more thing to note is that entries starting with the word "animation" is no longer present in the FreeFalcon 6 installation's particlesys.ini file. So this part of the code is not going to be executed any more.

At this point, I am wondering if we should try to use a lower version of FF to try with the code, may be FF5?

PlutoniumHeart commented 11 years ago

I am able to get into the game now, but only the GUI not the 3D environment though. There are a lot of errors though...

vinnydiehl commented 11 years ago

@PlutoniumHeart: Did downgrading to FF5 allow you to get into the GUI? I'm about to try that right now.

Edit: Nope, that didn't do it, I'll assume that you hacked away a bunch of roadblocks to get in. : P

PlutoniumHeart commented 11 years ago

No, I modfied the bool DrawableParticleSys::LoadParameters(void)...

However, I just noticed that whoever wrote this function already realized that this is an outdated function so they defined something like this: it turns out the bool DrawableParticleSys::LoadParameters(void) is outdated, the code defines something like #ifdef USE_NEW_PS return PS_LoadParameters();

else

but the macro somehow doesn't work, even if you manually define it before the function call, so I removed everything so that the new function can be called. Unforturnately, this new function doesn't work either, I am looking into how it handles the word "trail" now, so that I can modify it to work.

PlutoniumHeart commented 11 years ago

B the W, this piece of code in the DrawableParticleSys class is the worst piece code I have seem ever...

vinnydiehl commented 11 years ago

Haha, you'll probably be able to find worse in here. Check out what I had to do for 39786956ca79680a541b65cd26211e1b197eb849. :anguished:

I assigned you on this issue, queue up a patch if you think you're onto the problem. I gave you push access to this repository, so work on a branch and push it up to here (git push origin feature/particlesys-parse-fix or whatever, you'll have to change your remote to point to this one), select the branch, and Pull Request it, we'll discuss the patch.

PlutoniumHeart commented 11 years ago

Well, formating is one thing, I can tolerate that, it is the unprofessionality in the work flow that's driving me crazy... I would expect they put a line of "continue;" at the end of each iteration, but no, not even this simplest common sense.

PeterMeyer commented 11 years ago

The Particlesys Code is from some of the BMS Team. A few Days ago as we started he claimed its IP on the particlesys stuff hand how dare we are using such a advanced stuff ;d

PeterMeyer commented 11 years ago

Original Code.I have some but you must know i had to puzzle things together, because the former coder gave FF not all of his work back This Code reflects not the same FFViper.exe Version wich is shipped with the FreeFalcon 6.0 Installer. The code has maybe the featureset of the year 2009, therefore it should reflect more the FreeFalcon 5.x serires (the old Coder didnt give the FF Team all its work back) This might can explain something. But anyway. The old Code code was fixed to VS 2003. All the other FalconTeams out there was not able to port it in a compileable version over the cliff upto VS2005/2008/2012 because Microsoft had changes internal things, so they trapped and fixed to the buggy 7.1 Compiler until today. So we ahead a bit with our Toolchain Situation with the price tag thadt something needs to be fixed.

PlutoniumHeart commented 11 years ago

Man, I hope this guy is out of the BMS team, otherwise, BMS's code would be a disaster to maintain in the future.

Multifalcon commented 11 years ago

HI sorry to be out. but RL has priority.

I will try to be here soon. Cheers.

vinnydiehl commented 11 years ago

@Multifalcon: No problem, good luck with whatever you're dealing with.

Everyone else: Where should we go from here? Try to fix this mess, rewrite particlesys completely, or nuke it?

PlutoniumHeart commented 11 years ago

I am still working on this one, I think I'll try to clean it up soon.

vinnydiehl commented 11 years ago

@PlutoniumHeart: Alright. Thanks a lot for all of the time that you're putting into this, it's much appreciated.

PeterMeyer commented 11 years ago

@PlutoniumHeart Yeah absolutly awsome!

PeterMeyer commented 11 years ago

@gbchaosmaster We should fix it for now but this particlesys code is not the best in the world i can imagine (id dont want use strong words here) but we should use the libbullet (opensource physics lib) to get some advanced and really awsome physics results http://bulletphysics.org i think.

vinnydiehl commented 11 years ago

@PeterMeyer: My thoughts exactly, I like libbullet.

PeterMeyer commented 11 years ago

Awsome! For the Flightphysics of the Aircrafts i have this in my mind (free to choose FDM of cause) http://freefalcon.tk/topic/8418555/1/

vinnydiehl commented 11 years ago

Seeing as that's an entirely different issue than this one I'll take this discussion to that thread.

PlutoniumHeart commented 11 years ago

OK, I have "cleaned up" the particlesys part of the code. During the process, I banged my head against the wall multiple times, asking myself, was it worth it to clean up the mess? And why would anyone hire a 10-year-old for a project of more than 610000 C/C++ lines involved?

Bottom line is now you can enter the 2D environment after ignoring/acknowledging multiple additional errors. However, you can't get into the 3D environment yet because of this, and when I say I "cleaned up" the particlesys part I really meant that I managed to run pass it, the memory operation in this piece of shXt is at best a mess. (who does a memset right after a new to the same pointer??) whoever did this mess clearly have no idea what he's doing with C style memset and C++ style new() delete(), so I can assure you there are un managed memory locates inside, I just don't have that much time to find and fix it.

Another good example of this piece of awesome code is that he used a for loop in the following way: for (t = 0; t < PSMAX_EMITTERS && PS_PPN[c].emitter[t].stages; t++) { /more of his junk here/ } When I read this, I was larghing so hard that I think I peed a little...

At last, I would like to know how should I commit this code since you guys pulled me into othis project. And if any of you guys are friends with this guy who wrote this piece of code, please don't be offended, I am extremely sorry to rant about my headache in public, although I believe after cleaning someone's massive screwed-up-piece-of-work, I am entitled to do so.

Plutonium.

PlutoniumHeart commented 11 years ago

Now in a more serious note, I hope you guys can have a better time making this work. I would recommend not to make major changes to existing features before we can make it work as it intended to.

There are weird errors from Dialog box about "bad maneuver data file format", you guys should notice it immediately after you got my modified code.

Thanks!

Plutonium

vinnydiehl commented 11 years ago

Hey @PlutoniumHeart, thanks a lot for your work, and nope, I don't know the guy and if I did I'd have a few stern words to say about the shit code that he's been writing, heh. Anyway, now that you're part of the team, you can take the following steps to do commits to the project.

  1. Change your origin remote to point to this repo (git remote set-url origin git@github.com:FreeFalcon/freefalcon-central.git), you'll only have to do this once
  2. Branch off of the develop branch so you have a place to safely work in (git checkout -b feature/name-of-feature-or-whatever develop)
  3. Write your code, making regular commits. Try to limit each commit to one unit of change: one fix, one feature, etc. Check the diffs before you commit to make sure that you're only committing what's necessary- look into git add -p if you want to selectively commit, plenty reading on Google about it
  4. While writing your code, push that branch up to the main repo and create a pull request for it (by switching to the branch on GitHub and hitting the Pull Request button), even if it isn't ready to merge yet (just say so if it isn't), that way we can all work together on it
  5. Once we all decide that the code is stable, I'll merge everything into the main develop branch and close the issue

And I think I already mentioned this, but never commit commented-out code, just delete it, Git will keep it safe in the history in case we ever want it back.

I noticed the bad maneuver data file format error, but didn't look too far into it because I wanted to try to focus on this bug. That'll be the next to stomp.

Vinny

PlutoniumHeart commented 11 years ago

OK, now I just found out that you have to put the _filbuf() function back, but rename, otherwise the sim won't be able to read zipped files. And in addition to that, the "mnvrdata.dat" file in zip file is not the correct version, not even close. The sim is not able to process it, we need to see the old or new version of this file, other wise, the code would has to be rewrote.

vinnydiehl commented 11 years ago

That's a completely separate issue from this, no? Should probably make the fixes for this overflow and the mnvrdata failure separate issues/pulls.

I only have the game files for FF5.5, 5.55, 6.0, and maybe 5.0 if I'm lucky. I'd have to go spelunking to get any earlier version. Not sure what versions the other guys have available.

PeterMeyer commented 11 years ago

I have some old Code but you need VS2003 to get it work.I will see how i can make it avaiable for you. But if you can rewrite it, it would be the better Solution Plutonium. I send you guys a pm with the link of the untouched code, original code i was sended via email as i joined the FF Team in 2012

PlutoniumHeart commented 11 years ago

@gbchaosmaster I was not able to run the second command, and I think I just pushed the commit under the origin, hope that's fine.