Missing model but shadow rendered

[illwieckz]

Disclaimer: this has been observed on an experimental branch for running the game on ppc64el.

I observed the bug in that video: https://youtu.be/lJPENFtjVgU?t=794

Quote:

invisible-dretch.mp4

While the build is experimental, it was running the vanilla 0.56.1 game code on both sgame and cgame, and the only difference with the released engine aren't related to models.

So we can't exclude a bug specific to the architecture, but at the same time it looks very close to the “model not displayed when wallwalking” bug that was fixed just before release, so maybe that's a remaining trace of it or something similar.

In the video, the model is invisible when appearing in front of the player, so maybe it's related to how entities becomes visible, and it may be related to the entity cache. Though, since the shadow is drawn, some entity position seems to be known. But maybe only the shadow mark is known, not the model yet.

@VReaperV may have some idea on the topic.

[VReaperV]

In the video, the model is invisible when appearing in front of the player, so maybe it's related to how entities becomes visible, and it may be related to the entity cache. Though, since the shadow is drawn, some entity position seems to be known. But maybe only the shadow mark is known, not the model yet.

@VReaperV may have some idea on the topic.

You force pushed that branch a bunch, then merged it of your own volition after claiming that you tested it, so I cannot with any certainty say whether those changes could've caused this issue.

[illwieckz]

What I modified was just committing submodules. I verified commits one by one.

[illwieckz]

I verified each commit by exporting them to .patch files and diffing them.

[illwieckz]

I also rebased them as they were based on an old branch. I verified commits one by one by diffing them after the rebase.

[slipher]

Before the skeleton changes were merged, there was a similar bug that sometimes happened happened the first frame an entity is visible. Likewise the shadow was drawn, but the model was not visible, or sometimes visible but the wrong size and distorted. So in that case it was trying to draw the model but the skeleton was incorrect which made it sometimes impossible to see.

[illwieckz]

Here is the backup of your engine branch:

• https://github.com/DaemonEngine/Daemon/commits/bkp/reaper-engine-skeleton/

Here are the commits of your engine branch and the one I rebased, dumped as patches:

• daemon-skeleton-diff.zip

Here is the diff of all those engine patches:

$ diff a1.patch b1.patch 
1c1
< commit 72b2d17319f507549fa6af29fca8bd4bbf1ce94c
---
> commit a70030686f76da62730b50ff17ebf5d4f4d573cc

$ diff a2.patch b2.patch 
1c1
< commit 46a2eb3a35f339cc16dee07953b8c1494805b10b
---
> commit 5b15272aa68bb5cd5b8e382aa497496b53c31d3e

$ diff a3.patch b3.patch 
1c1
< commit 0389e6f985bb2f8f66fcbe1dd482b1161d103ba9
---
> commit 2345c19c18d261f5f296e92d3e9c60a2a9736ab0

$ diff a4.patch b4.patch 
1c1
< commit 8092369140d5641620e6cba78a6c897280995f35
---
> commit b4c6e33d1f966bf8b42f1f0ca4867d78fbfb7c65
712c712
< index f1921b96c..2b6c2791b 100644
---
> index 92a738125..3c66a2d24 100644
724c724
< @@ -1843,9 +1845,9 @@ static void RB_RenderDebugUtils()
---
> @@ -1883,9 +1885,9 @@ static void RB_RenderDebugUtils()
737c737
< index 8d496d1b4..e5a13fb05 100644
---
> index 321253dbc..b6174545d 100644
748c748
< @@ -1317,6 +1318,8 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
---
> @@ -1337,6 +1338,8 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
757c757
< @@ -1666,12 +1669,12 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
---
> @@ -1686,12 +1689,12 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
773c773
< index 68328fde7..748522d89 100644
---
> index 2a1f6513c..9d58e99b3 100644
822c822
< @@ -3325,6 +3328,7 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3335,6 +3338,7 @@ void GLimp_LogComment_( std::string comment );
830c830
< @@ -3365,6 +3369,7 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3375,6 +3379,7 @@ void GLimp_LogComment_( std::string comment );
838c838
< @@ -3589,11 +3594,10 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3599,11 +3604,10 @@ void GLimp_LogComment_( std::string comment );
853c853
< @@ -3606,6 +3610,8 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3616,6 +3620,8 @@ void GLimp_LogComment_( std::string comment );

The diff of the 4th commit is noisy because the engine branch wasn't on the latest for-0.56.0/sync branch so line numbers were different. The diff is only about different line numbers.

[illwieckz]

And here is the backup of your game branch:

• https://github.com/Unvanquished/Unvanquished/commits/bkp/reaper-engine-skeleton/

$ git diff 26b7bcdf3475883bd2e2d9994163857296168de9..b803ea46531a8240dee3b9863fa97f5e3711ad6a > a.patch
$ git diff 0b6425faf449186609501974dcd1d658b922343b..a83896bf5bc6f62f89df6d1f1c7d8459f3090761 > b.patch
$ git diff a.patch b.patch | wc -l
0

[illwieckz]

Here is the backup of your engine branch:

• https://github.com/DaemonEngine/Daemon/commits/bkp/reaper-engine-skeleton/

I pushed the wrong backup (it had your original commits, but not all of them), now it is the correct backup, with all your commits.

The check was done on the right commits though, as you can see me in the comparison in the above comment, comparing my rebased commit with 72b2d17 which is yours.

Just redoing the check because why not:

$ git diff 209a6eddbe15573ca46987761cecdf710285ef55..72b2d17319f507549fa6af29fca8bd4bbf1ce94c > a.patch
$ git diff f6251c7ff51c936780e14369a5e4916f1ac1f350..a70030686f76da62730b50ff17ebf5d4f4d573cc > b.patch
$ diff a.patch b.patch 
725c725
< index f1921b96c..2b6c2791b 100644
---
> index 92a738125..3c66a2d24 100644
737c737
< @@ -1843,9 +1845,9 @@ static void RB_RenderDebugUtils()
---
> @@ -1883,9 +1885,9 @@ static void RB_RenderDebugUtils()
750c750
< index 8d496d1b4..e5a13fb05 100644
---
> index 321253dbc..b6174545d 100644
761c761
< @@ -1317,6 +1318,8 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
---
> @@ -1337,6 +1338,8 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
770c770
< @@ -1666,12 +1669,12 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
---
> @@ -1686,12 +1689,12 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
786c786
< index 68328fde7..748522d89 100644
---
> index 2a1f6513c..9d58e99b3 100644
835c835
< @@ -3325,6 +3328,7 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3335,6 +3338,7 @@ void GLimp_LogComment_( std::string comment );
843c843
< @@ -3365,6 +3369,7 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3375,6 +3379,7 @@ void GLimp_LogComment_( std::string comment );
851c851
< @@ -3589,11 +3594,10 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3599,11 +3604,10 @@ void GLimp_LogComment_( std::string comment );
866c866
< @@ -3606,6 +3610,8 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3616,6 +3620,8 @@ void GLimp_LogComment_( std::string comment );

Yet again, onky line changes because of the rebase, other unrelated changes having been merged somewhere in an higher position of the file.

Just as a remind, GitHub retains the trace of the commits you pushed and still print their references.

On engine side, your last commit is 7122b17:

• 72b2d17

On Game side, your last commit is b803ea4:

• Unvanquished/Unvanquished@b803ea4

Those two commits are the HEAD of the bkp/reaper-engine-skeleton branches on engine side and game side:

• https://github.com/DaemonEngine/Daemon/commits/bkp/reaper-engine-skeleton
• https://github.com/Unvanquished/Unvanquished/commits/bkp/reaper-engine-skeleton

@VReaperV My force push was just me rebasing your code before merging, I have not introduced any code change. This rebasing also fixed the CI failure you were reporting. On game side the CI didn't fail “because it pulled for-0.56 instead of the correct engine branch”, it failed “because it attempted to rebase the game on for-0.56 and engine submodule commit was conflicting because of the missing rebase on engine side”. Once I did that rebase properly on both game and engine, the CI worked, and there was no merge conflict to solve.

The only thing I did was to rebase on latest engine and game trees and to commit some submodules along the way so future bisect don't break. I checked multiple times in multiple ways that the rebase didn't introduced any change by diffing commits one by one. I tested the build commit per commit, with their submodules updates. Then I pushed the rebased branches and merged this.

This is your code, unmodified, just rebased on what were latest for-0.56 branches at the time.

[VReaperV]

Here is the backup of your engine branch:

• https://github.com/DaemonEngine/Daemon/commits/bkp/reaper-engine-skeleton/

I pushed the wrong backup (it had your original commits, but not all of them), now it is the correct backup, with all your commits.

The check was done on the right commits though, as you can see me in the comparison in the above comment, comparing my rebased commit with 72b2d17 which is yours.

Just redoing the check because why not:

$ git diff 209a6eddbe15573ca46987761cecdf710285ef55..72b2d17319f507549fa6af29fca8bd4bbf1ce94c > a.patch
$ git diff f6251c7ff51c936780e14369a5e4916f1ac1f350..a70030686f76da62730b50ff17ebf5d4f4d573cc > b.patch
$ diff a.patch b.patch 
725c725
< index f1921b96c..2b6c2791b 100644
---
> index 92a738125..3c66a2d24 100644
737c737
< @@ -1843,9 +1845,9 @@ static void RB_RenderDebugUtils()
---
> @@ -1883,9 +1885,9 @@ static void RB_RenderDebugUtils()
750c750
< index 8d496d1b4..e5a13fb05 100644
---
> index 321253dbc..b6174545d 100644
761c761
< @@ -1317,6 +1318,8 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
---
> @@ -1337,6 +1338,8 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
770c770
< @@ -1666,12 +1669,12 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
---
> @@ -1686,12 +1689,12 @@ ScreenshotCmd screenshotPNGRegistration("screenshotPNG", ssFormat_t::SSF_PNG, "p
786c786
< index 68328fde7..748522d89 100644
---
> index 2a1f6513c..9d58e99b3 100644
835c835
< @@ -3325,6 +3328,7 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3335,6 +3338,7 @@ void GLimp_LogComment_( std::string comment );
843c843
< @@ -3365,6 +3369,7 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3375,6 +3379,7 @@ void GLimp_LogComment_( std::string comment );
851c851
< @@ -3589,11 +3594,10 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3599,11 +3604,10 @@ void GLimp_LogComment_( std::string comment );
866c866
< @@ -3606,6 +3610,8 @@ void GLimp_LogComment_( std::string comment );
---
> @@ -3616,6 +3620,8 @@ void GLimp_LogComment_( std::string comment );

Yet again, onky line changes because of the rebase, other unrelated changes having been merged somewhere in an higher position of the file.

Just as a remind, GitHub retains the trace of the commits you pushed and still print their references.

On engine side, your last commit is 7122b17:

• 72b2d17

On Game side, your last commit is `b803ea4`:

• Unvanquished/Unvanquished@b803ea4

Those two commits are the `HEAD` of the `bkp/reaper-engine-skeleton` branches on engine side and game side:

• https://github.com/DaemonEngine/Daemon/commits/bkp/reaper-engine-skeleton
• https://github.com/Unvanquished/Unvanquished/commits/bkp/reaper-engine-skeleton

@VReaperV My force push was just me rebasing your code before merging, I have not introduced any code change. This rebasing also fixed the CI failure you were reporting. On game side the CI didn't fail “because it pulled for-0.56 instead of the correct engine branch”, it failed “because it attempted to rebase the game on for-0.56 and engine submodule commit was conflicting because of the missing rebase on engine side”. Once I did that rebase properly on both game and engine, the CI worked, and there was no merge conflict to solve.

The only thing I did was to rebase on latest engine and game trees and to commit some submodules along the way so future bisect don't break. I checked multiple times in multiple ways that the rebase didn't introduced any change by diffing commits one by one. I tested the build commit per commit, with their submodules updates. Then I pushed the rebased branches and merged this.

This is your code, unmodified, just rebased on what were latest for-0.56 branches at the time.

You said you tested it, then emphasised in another issue how much time you spent testing 0.56, so I assume you wouldn't have missed something like this.

[illwieckz]

You shouldn't make such assumption because we are not gods.

Indeed yes, a lot of testing was done, but not everything can be spotted. The alternative would be to never release anything.

I do remind you I spotted many other things, which is a proof of the testing I have done.

Unvanquished is a very big project, so there are a lot of things to test. We were only a very few people testing, despite me having done a release candidate cycle of one month. So, I thank you for pointing out the fact we need the help from more testers.