Rigs-of-rods: Incorrect actor spawning on some terrains

Created on 22 Mar 2019  路  15Comments  路  Source: RigsOfRods/rigs-of-rods

Barges (20KLpont) on BDM3-20Kleagues:
screenshot_2019-03-21_21-27-27_1
Penguinville with trolley line's slidenode track:
screenshot_2019-03-21_21-30-48_1
RaceMax:
screenshot_2019-03-23_19-19-39_1

bug

Most helpful comment

All 15 comments

Would there be any big side effects if we just reverted that commit?

that was @ulteq's hardest work, im sure there is a way to fix it

@tritonas00 Thanks for the research, I guess Ulteq thought those transformations are not used by anyone anymore.

I sense we might need unit tests for these cases, to both check for regressions like this and to document what exactly is the expected behavior, including all the quirks. UPDATE: I'm looking at http://kaizou.org/2014/11/gtest-cmake/

Anyway, I'll check the Barges now. UPDATE: I experimentally reverted the offending commit c2dfa46 and it works - but I want to keep the commit, so I'll use the working branch to trace the glitch and fix upstream.

BUMP!

I tried creating automated tests for RoR and it looks very very promising: https://github.com/only-a-ptr/rigs-of-rods/tree/fix-spawn-positions/test. So, I'm delaying the release some more, but I think it's well worth it.

Immediate goal: To develop automated check that spawning actor from '.tobj' file (terrain) with specific position+rotation will result in expected node positions.

Future goal: To develop automated checks for all data transforms done by RoR: spawning, forces, collisions, friction, networking, everything that's so easy to throw off.

Status: _UPDATE: 2019-07-18_ Right now everything is in one experimental dev branch, it's a big mess but it keeps working better and better, I'm pouring code to it on every occasion and learning as I go. Breakdown:

  • UtilsTestSuite: contains one unit test of SanitizeUtf8String() - checks that invalid chars are correctly replaced. Up to date.
  • ActorTestSuite: contains one unit test of Actor::ResetPosition() - second test I made (experiment), up to date.
  • SimControllerTestSuite: WIP, the one to check the terrn-spawning,

BUMP! What a nuissance - I just discovered another bug which existed before c2dfa46 (NHelens bridge is misplaced by a few meters). To fix all the glitches without messing it any further, I need to find a commit where both things worked fine - @tritonas00 @CuriousMike56 Can you help?

nhelens updated for https://github.com/RigsOfRods/rigs-of-rods/commit/c2dfa46e785a4f8803d3be207a85609be6026021 after, mike can revert the change again

Oh, well, that's a good way to drive a developer insane. Can you link me to an un-modified nhelens please?

Also it further proves that it's about time I created the automated tests - if the content keeps changing to accomodate new glitches in RoR, how else can we tell if we're still backwards compatible?

reverting it will make things as backwards compatible as possible

and iirc only nhelens updated

@CuriousMike56 can help with the previous (original) version of nhelens

I'm glad to hear there weren't many changes yet. My plan:

  1. Revert c2dfa46 to have perfectly backwards-compatible code.
  2. Incorporate tests from experimental branch
  3. Re-apply c2dfa46 and use the tests to detect and fix any glitch

try this https://archives.rigsofrods.net/repo/files/repofiles-1st-batch/959aa358c2c57e54eea789eae8a7e4eb90bca0dd_nhelens.zip

are you sure those tests are going to work?

just reverting https://github.com/RigsOfRods/rigs-of-rods/commit/c2dfa46e785a4f8803d3be207a85609be6026021 for 0.4.8 should be good

then research/test/etc.. would be better. It is taking too long, and we have 2-3 issues left, which im sure you could have easily fix them meanwhile

Well, it's software, bugs can happen. Howerver, it'll be in Git with a revision history and sooner or later, we'll cover all possible scenarios.

Just take a look at the WIP 20KLpont test: https://github.com/only-a-ptr/rigs-of-rods/blob/fix-spawn-positions/test/suites/SimControllerTestSuite/SimControllerTestSuite.cpp#L40-L107

Research+test is not good enough anymore. Ulteq broke it because the code is so convoluted that even he didn't get the research right. Also, the problem was tricky to detect because it didn't affect trucks which have node zero at coordinates 0,0,0 (it's not required but many trucks have it, for example scaniawrecker on nhelens). So, the tester would have to spend a lot of time testing all possible content (and watch out for updated content). That works now thanks to you, but eventually life will happen and you won't have time/energy anymore.

just revert the code and don't touch it again isn't an option i guess? :laughing:

we should have test this better back then :woman_facepalming:

The tests are complete, I just need a positively bug-free code to find valid outputs for test inputs. Now that you provided it (by explaining the nhelens bridge stunt), I'll finish it in a matter of days.

Yes I could just revert it now and do the rest later, but I really really want to be done with it for good. Going back to finish something is super-annoying.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

KinG-InFeT picture KinG-InFeT  路  3Comments

ghost picture ghost  路  3Comments

tritonas00 picture tritonas00  路  9Comments

WillyB1337 picture WillyB1337  路  4Comments

4x4convoy picture 4x4convoy  路  6Comments