Atlas dll outdated (Actor Viewer doesn't play animations anymore)

[elexis]

On August 7th, AlexanderMB reported to me that the Atlas Actor Viewer doesn't play animations anymore.

This is reproducible on Windows, but not on linux.

I don't see a suspicious atlas commit, so I guess it is related to the code that changed unit animations.

[elexis]

I've done some bisecting:
r22245 plays the animations correctly
r22345 plays the idle animation always

[elexis]

As the bug seems to be playing always the idle animation, I suppose it's related to the commits that changed playing the idle animation. So perhaps it was r22475 or related.

[nani]

On svn revision 22335 works, 22344 doesn't. Windows, Visual Studio build.

[elexis]

According to my testing it works with r22334 and r22335 broke it.

[s0600204]

As the author of the offending commit, I shall try to resolve the issue. However, I do not have a Windows machine, thus I'm working a little blind. Could I ask someone who does have access to Windows to help test things?

Assumptions

1. That the list of animations is being populated correctly, e.g. the names of the animations appear in the dropdown.

2. That the Idle animation does indeed play, e.g. the entity moves when the "Play" button below the dropdown is pressed.

3. That there are no error, warning, caution, or debug messages posted to the console line, nor the pyrogenesis log file.

Tracing

When a user selects an animation, it is handled by ObjectBottomBar::OnSelectAnim() (​Object.cpp#612) where the string is extracted (from the selection event of the dropdown) and stored in m_ActorViewerAnimation.

Then ActorViewerPostToGame() (​line 118) gets called which "posts" a SetActorViewer message with the string's contents.

(This method is also called when the ActorViewer is initialised, but with the animation set to the string idle. We could theorise that as the idle animation plays after the initialisation, that the code from this point on must be functioning. However this theory could be incorrect, as the ​unit animation system also defaults to idle.)

The "posted" message is picked up by a handler in GraphicsSetupHandlers.cpp (​line 154).

This handler passes the animation name to SetActor() in ActorViewer.cpp (​line 332), where it is converted to lower-case, and passed on to the Interface for the VisualActor component.

The code from this point onwards hasn't been changed by the offending commit, so we can safely assume that that is all still working okay.

Theory

Most of the changes along the traced path are changing from wide-strings to narrow ones, and I'd like to think that if I'd missed one or got it wrong the compiler would complain. Loudly. (And if not, someone on the team no doubt would.)

So I'm drawn to the changes that are not the above, and as such my current working theory is that the string is not being read correctly from the dropdown (and in such a way that it fails silently).

We don't actually state how the conversion from wxString to std::string should be achieved, thus the conversion is implicit. The wxwidgets guide provides the following ​sage advice:

[The] wxString API provides implicit conversion of the internal Unicode string contents to narrow, char strings. This can be very convenient [...] however it is a rather dangerous operation as it can easily give unexpected results if the string contents isn't convertible to the current locale.

The animation names are all basic ascii letters (A-Za-z) with _ (underscore) being the only non-alphabetic character used, so I have doubts that they wouldn't convert to the "current locale" of Windows, but what the heck: fortune favours the bold (and all that).

Trial

For my first attempt, I'd like to try explicitly stating how the conversion should be done. So could I ask someone to try both of the following and seeing if either work on Windows?

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.cpp

@@ -611 +611 @@
 
 void ObjectBottomBar::OnSelectAnim(wxCommandEvent& evt)
 {
-       p->m_ActorViewerAnimation = evt.GetString();
+       p->m_ActorViewerAnimation = evt.GetString().ToStdString();
        p->ActorViewerPostToGame();
 }

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.cpp

@@ -611 +611 @@
 
 void ObjectBottomBar::OnSelectAnim(wxCommandEvent& evt)
 {
-       p->m_ActorViewerAnimation = evt.GetString();
+       p->m_ActorViewerAnimation = evt.GetString().utf8_str();
        p->ActorViewerPostToGame();
 }

Steps:

1. Apply one of the above patches;
2. Compile;
3. Open Atlas;
4. Switch to the ActorViewer;
5. Select an entity that supports various animations;
6. Set the animation to play;
7. Choose an animation that the selected entity supports;
8. Note the result;
9. Close Atlas;
10. Revert the applied patch;
11. Apply the other patch;
12. Repeat steps 2 through 10;
13. Report back here.

Thanks!

[elexis]

Thanks for having taken a look so far!

I've tested the two patches with VS2015, also added a LOGERROR, it seems ObjectBottomBar::OnSelectAnim is never called or it ignores even a std::terminate().

On linux it works.

I have no idea as to why this could be the case. I suppose it can be found out by bisecting within the commit.

[elexis]

I've tested yet again. The function is not called anymore on Windows, but on unix. I've added std::terminate() and std::exit(), LOGERROR, nothing is happening.

I've reverted this commit again (and r22161 for the merge conflict) using svn merge -c -r1234 . to the current working copy and it works again. Something is off.

[s0600204]

Chakakhan in #0ad-dev at 2019-09-04 23:39, said:

Testing #5574 - Running from the command line, animations don't play.

But running in the debugger, they DO play!

It takes forever for the actor viewer to load, but the animations are working in the debugger.

(this is on windows 10)

I tested this both running Atlas.bat and from pyrogenesis... neither work. Running pyrogenesis in the debugger and loading the scenario editor from there works.

Well, thank you for trying, both of you.

---

If it wasn't for the fact that the animation names appear in the widget, then I'd say it wasn't converting the strings into wxString correctly.

But now apparently it works... but only when running through a debugger?!

I had been looking through the wxwidgets code to try and see if I can spot anything that might be amiss... I may have been looking in the wrong place.

Is this a compiler issue? A quirk of how we use wxwidgets?

I've low confidence that it'll prove anything, but: I've thrown together a small (and inelegant) program (based off the wxWidgets "Hello World" tutorial) that builds and displays four wxChoice widgets (each supplied with strings of different types) in much the same way that we do in Object.cpp. Upon selecting an entry from one of the widgets, said entry should appear in a messagebox.

I'd be interested to see if you can't get a response from any of the wxChoice widgets: ​https://code.wildfiregames.com/P168

[s0600204]

Chakakhan in #0ad-dev at 2019-09-05 02:07 UTC said:

Ok, so the test script works fine, both from the command line and the debugger, so I think this is not the string handling bits.

[Kenny Long]

Hey Guys, I was going to do some testing to see if I can narrow this down a bit between the last commit and the previous working commit. I found the source files in the source directory, but I don't see them in the VisualStudio pyrogenesis solution anywhere. Am I just missing them? Do I need to change these and compile them outside of VS? I am going to try reverting parts of the code until I get it to work. Any help is appreciated.

[elexis]

I didn't perform these steps: ​https://trac.wildfiregames.com/wiki/BuildInstructions#BuildingAtlas which explains why std::terminate() didn't do anything, it must have not been rebuilt!

[Kenny Long]

Yea, that fixed my problem. One note, if you run update-workspaces.bat without the --atlas flag it will remove the atlas projects... ask me how I know ;-)

[Kenny Long]

After updating work spaces with the --atlas flag, the actor animations are working in release mode (no debugger). So this is not a code issue. The question is, was this a build issue? Does this feature only work properly when built with the full atlas projects? Was this always this way? What else do I need to check to help get this ticket closed?

Thanks!

[elexis]

I've triggered a new complete autobuild with atlas, gloox and everything r22860, but there was r22681 already, so it sounds like Atlas autobuild is broken? Unless you tell me the new version works.

[Stan]

It's broken. No atlas Dll was committed.

[Kenny Long]

I downloaded a new clone from svn to a new directory and it is working for me. Not sure what is up here.

[elexis]

I reverted and updated and it still doesn't work for me (didnt compile so I can reproduce what other autobuild users see).

But I do see that if I change the animation dropdown item, it restart the idle animation. So the event is sent here too.

My last atlas dll is from r21616. So maybe the Atlas autobuild is broken since then as I definitely clicked the rebuild Atlas setting in Jenkins and I don't see other builds that look like they want to be triggered in that menu, and maybe it works for Chakakhan due to some way of finding the path to the dll on his system?

[Stan]

If he did a clean cloning of SVN and not a svn cleanup this seems unlikely. Windows only put specific dlls in Gac if you ask it to.

One could look at the jenkins (that is alledgedly versioned now) script and see where it fails.

[Kenny Long]

I did a clean cloning to a different directory. I did not compile, so it should be what was in the autobuild - and it worked for me. I'm on Windows 10.

[Kenny Long]

Ok, I installed it on another machine and it does not work. I rechecked my second install and it also did not work. I must have gotten my wires crossed. So now we are all consistently seeing the same behavior, indicating a build problem.

[elexis]

Angen for the rescue: ​Phab:D2021

[s0600204]

Unassign myself, as it seems I'm not much use here...

My thanks to those who have helped thus far.

[Silier]

I can play animations in atlas viewer after dll was rebuild rP22918

[Itms]

This was fixed in r22918, even though there was a remaining issue in the Jenkins pipeline.

[Itms]

In 23213:

Fix the Atlas build in the Windows autobuilder pipeline, refs #5574.
Use the opportunity to fix some whitespace and to make the file name consistent with Jenkins.

Patch by: Angen
Differential Revision: ​https://code.wildfiregames.com/D2021

[Silier]

Since jenkins script was confirmed to be working, it is likely not getting "atlas" variable set to true because DLL was again not rebuild after r23847 in r23848.

[Stan]

We won't do automatic atlas builds and it will likely always need to be triggered by team members. The dll has been updated today, so the issue is fixed.

Relevant Thread: ​https://wildfiregames.com/forum/index.php?/topic/28539-atlasdll-autobuild/&tab=comments#comment-401318