OpenClonk Bugtracker - OpenClonk
View Issue Details
0000931OpenClonkObjectspublic2013-03-01 20:082017-10-25 05:52
ReporterPyrit 
Assigned ToZapper 
PrioritylowSeverityminorReproducibilityalways
StatusassignedResolutionopen 
PlatformPCOSWindows 7OS Version64 bit
Product Version 
Target Version9.0Fixed in Version 
Summary0000931: Buildings shake when being built
DescriptionWhen a building builds itself up, it jiggles up and down.
Steps To ReproduceBuild something
Additional InformationThis occured to me in CR, too.
It may have something to do with the position of the building not being displayed right. Maybe the enngine has difficulties to position half completed buildings right, since they don't have their full size?
TagsNo tags attached.
Attached Filespatch c4shape-use-c4real.patch (25,659) 2015-10-29 20:47
https://bugs.openclonk.org/file_download.php?file_id=518&type=bug

Notes
(0003902)
Clonkonaut   
2015-10-12 18:56   
There are probably some position errors / rounding things when the shape of an object grows that lead to be graphics being drawn one pixel afar.
I am not sure this is worth the effort to fix. It's a very minor visual thing.

Set the priority to low.
(0004061)
Zapper   
2015-10-29 20:46   
I actually spent some time on this today. The reason is that DoCon adjusts for the bottom-most vertex. But the vertices are not in C4Real coordinates like the object position but in int32_t.

That means that the position does get rounded and thus jumps up and down.

I tried making C4Shape use C4Real for the vertices & shape (which worked). But the values that are loaded from the DefCore are still integers. I might finish that some day when I want to dive into how mkNamingAdapt works, meh.

There might still be some issues then (e.g. with loading old savegames?). No idea.
I'll just attach the work-in-progress patch here so it doesn't get lost. The todo is that the C4Reals in C4Shape actually just contain the raw integers that are read from the DefCore. No idea why, I'd expect that those integers are passed into C4Real::C4Real(int) and then properly converted. But maybe the StdCompiler does some extremely weird stuff or just cannot read C4Real or something from the DefCore format. Idk.
(0004062)
Sven2   
2015-10-29 22:29   
(Last edited: 2015-10-29 22:36)
C4Real aka FIXED is saved as the "raw" (i.e. *65536) integer. That's just how the corresponding CompileFunc works. Of course you can easily work around that by making a mkParAdapt allowing you to load and/or save them in a different format for definitions. For objects they should stay in the current format because if they're saved rounded to int, there will be sync losses e.g. on runtime join.

Concerning the "shaking", I don't think it would be solved by making vertices C4Real. There would still be shaking because they have to be rounded when they're doing the landscape check (either that or DoCon will silently push the bottom vertex into a stuck state).

Anyway, there might be a much simpler solution: Just adjust the object position smoothly such that the bottom vertex is always at the same position. That shouldn't be too hard to calculate at least for un-rotated objects.

(0004063)
Sven2   
2015-10-29 23:32   
(Last edited: 2015-10-29 23:33)
Ah, unfortunately the simple solution does not work because gravity will always kill the smoothly pushed sub-pixel position (the same effect can be observed for vertically moving SolidMasks e.g. on the blimp).

So Zapper's proposed solution (plus sub-pixel movement of the object during construction) seems to be the only way. But when changing something so deeply in the movement code, I fear there may be a lot of possible, unexpected consequences.

(0004066)
Zapper   
2015-10-30 19:56   
I believe we should postpone that to 8.0.

There is probably not much missing from my patch above, but due to the possibility of new bugs we should do it for a fresh release!
(0004067)
Sven2   
2015-10-30 21:16   
Agreed. Pushed to 8.0.
(0005799)
Clonkonaut   
2017-10-24 22:04   

Reminder sent to: Zapper

What about your patch?
(0005800)
Zapper   
2017-10-25 05:52   
The same as just before the release of 7.0:
The patch might introduce new bugs, so we should wait until after the release to apply it - and this time really do it, that's why I assigned it to me :)

Issue History
2013-03-01 20:08PyritNew Issue
2015-10-12 18:53ClonkonautPrioritynormal => low
2015-10-12 18:56ClonkonautNote Added: 0003902
2015-10-12 18:56ClonkonautStatusnew => acknowledged
2015-10-16 00:32Sven2Target Version => 7.0
2015-10-29 20:46ZapperNote Added: 0004061
2015-10-29 20:47ZapperFile Added: c4shape-use-c4real.patch
2015-10-29 22:29Sven2Note Added: 0004062
2015-10-29 22:32Sven2Note Edited: 0004062bug_revision_view_page.php?bugnote_id=4062#r988
2015-10-29 22:32Sven2Note Edited: 0004062bug_revision_view_page.php?bugnote_id=4062#r989
2015-10-29 22:32Sven2Note Edited: 0004062bug_revision_view_page.php?bugnote_id=4062#r990
2015-10-29 22:33Sven2Note Edited: 0004062bug_revision_view_page.php?bugnote_id=4062#r991
2015-10-29 22:36Sven2Note Edited: 0004062bug_revision_view_page.php?bugnote_id=4062#r992
2015-10-29 23:32Sven2Note Added: 0004063
2015-10-29 23:33Sven2Note Edited: 0004063bug_revision_view_page.php?bugnote_id=4063#r994
2015-10-30 19:56ZapperNote Added: 0004066
2015-10-30 21:16Sven2Note Added: 0004067
2015-10-30 21:16Sven2Target Version7.0 => 8.0
2017-08-20 11:35ZapperAssigned To => Zapper
2017-08-20 11:35ZapperStatusacknowledged => assigned
2017-08-20 11:35ZapperTarget Version8.0 => 9.0
2017-10-24 22:04ClonkonautNote Added: 0005799
2017-10-25 05:52ZapperNote Added: 0005800