New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bugfix] Fixes crash when entering the skeletonview on a coupled train #578

Merged
merged 1 commit into from Jan 15, 2016

Conversation

Projects
None yet
2 participants
@ulteq
Contributor

ulteq commented Jan 14, 2016

Prevents asynchronous calls to updateSimpleSkeleton()

[Bugfix] Fixes crash when entering the skeletonview on a coupled train
Prevents asynchronous calls to updateSimpleSkeleton()
@only-a-ptr

This comment has been minimized.

Show comment
Hide comment
@only-a-ptr

only-a-ptr Jan 15, 2016

Nice catch.

Anyway, now that you touched the subject, the skeletonview could use some cleanup.

What is bool Beam::lockSkeletonchange? IMO it's a dirty workaround for broken [show|hide]Skeleton() handling - the functions are probably called multiple times upon the same vehicle.

A clean architectural solution would be:

  • Let the simthread set a per-vehicle "skeletonview_is_active" flag. It has to be simthread because skeletonview depends on hook connections, and those are updated on simthread. Duplicated setting of flag to the same value doesn't really matter 😄
  • Let the mainthread loop through vehicles and update skeletonview-related OGRE objects.

only-a-ptr commented on 39f001b Jan 15, 2016

Nice catch.

Anyway, now that you touched the subject, the skeletonview could use some cleanup.

What is bool Beam::lockSkeletonchange? IMO it's a dirty workaround for broken [show|hide]Skeleton() handling - the functions are probably called multiple times upon the same vehicle.

A clean architectural solution would be:

  • Let the simthread set a per-vehicle "skeletonview_is_active" flag. It has to be simthread because skeletonview depends on hook connections, and those are updated on simthread. Duplicated setting of flag to the same value doesn't really matter 😄
  • Let the mainthread loop through vehicles and update skeletonview-related OGRE objects.
@ulteq

This comment has been minimized.

Show comment
Hide comment
@ulteq

ulteq Jan 15, 2016

Contributor

the skeletonview could use some cleanup

Agreed, but in a separate pull request.

Contributor

ulteq commented Jan 15, 2016

the skeletonview could use some cleanup

Agreed, but in a separate pull request.

@only-a-ptr

This comment has been minimized.

Show comment
Hide comment
@only-a-ptr

only-a-ptr Jan 15, 2016

Member

Agreed, but in a separate pull request.
OK.

Member

only-a-ptr commented Jan 15, 2016

Agreed, but in a separate pull request.
OK.

only-a-ptr added a commit that referenced this pull request Jan 15, 2016

Merge pull request #578 from ulteq/skeletonViewCrashFix
[Bugfix] Fixes crash when entering the skeletonview on a coupled train

@only-a-ptr only-a-ptr merged commit 7e2c8e0 into RigsOfRods:master Jan 15, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ulteq ulteq deleted the ulteq:skeletonViewCrashFix branch Jan 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment