Opened 5 years ago
Closed 4 years ago
#5574 closed defect (fixed)
Atlas dll outdated (Actor Viewer doesn't play animations anymore)
Reported by: | elexis | Owned by: | |
---|---|---|---|
Priority: | Release Blocker | Milestone: | Alpha 24 |
Component: | Build & Packages | Keywords: | |
Cc: | Patch: | Phab:D2021 |
Description
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.
Change History (28)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
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.
comment:3 by , 5 years ago
On svn revision 22335 works, 22344 doesn't. Windows, Visual Studio build.
comment:5 by , 5 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
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
- That the list of animations is being populated correctly, e.g. the names of the animations appear in the dropdown.
- That the Idle animation does indeed play, e.g. the entity moves when the "Play" button below the dropdown is pressed.
- 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
a b void ObjectBottomBar::OnViewerSetting(wxCommandEvent& evt) 611 611 612 612 void ObjectBottomBar::OnSelectAnim(wxCommandEvent& evt) 613 613 { 614 p->m_ActorViewerAnimation = evt.GetString() ;614 p->m_ActorViewerAnimation = evt.GetString().ToStdString(); 615 615 p->ActorViewerPostToGame(); 616 616 }
-
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.cpp
a b void ObjectBottomBar::OnViewerSetting(wxCommandEvent& evt) 611 611 612 612 void ObjectBottomBar::OnSelectAnim(wxCommandEvent& evt) 613 613 { 614 p->m_ActorViewerAnimation = evt.GetString() ;614 p->m_ActorViewerAnimation = evt.GetString().utf8_str(); 615 615 p->ActorViewerPostToGame(); 616 616 }
Steps:
- Apply one of the above patches;
- Compile;
- Open
Atlas
; - Switch to the
ActorViewer
; - Select an entity that supports various animations;
- Set the animation to play;
- Choose an animation that the selected entity supports;
- Note the result;
- Close
Atlas
; - Revert the applied patch;
- Apply the other patch;
- Repeat steps 2 through 10;
- Report back here.
Thanks!
comment:6 by , 5 years ago
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.
comment:7 by , 5 years ago
Keywords: | regression added |
---|
comment:8 by , 5 years ago
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.
comment:9 by , 5 years ago
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
comment:10 by , 5 years ago
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.
comment:11 by , 5 years ago
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.
comment:12 by , 5 years ago
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!
comment:13 by , 5 years ago
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 ;-)
comment:14 by , 5 years ago
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!
comment:15 by , 5 years ago
comment:16 by , 5 years ago
It's broken. No atlas Dll was committed.
Edit the previous one was also broken.
comment:17 by , 5 years ago
I downloaded a new clone from svn to a new directory and it is working for me. Not sure what is up here.
comment:18 by , 5 years ago
I reverted and updated and it still doesn't work for me.
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?
comment:19 by , 5 years ago
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.
comment:20 by , 5 years ago
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.
comment:21 by , 5 years ago
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.
comment:22 by , 5 years ago
Component: | Atlas editor → Build & Packages |
---|---|
Keywords: | regression removed |
Patch: | → Phab:D2021 |
Summary: | Atlas Actor Viewer doesn't play animations anymore → Atlas dll outdated (Actor Viewer doesn't play animations anymore) |
Angen for the rescue: Phab:D2021
comment:23 by , 5 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Unassign myself, as it seems I'm not much use here...
My thanks to those who have helped thus far.
comment:25 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
This was fixed in r22918, even though there was a remaining issue in the Jenkins pipeline.
comment:27 by , 4 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:28 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
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
I've done some bisecting:
r22245 plays the animations correctly
r22345 plays the idle animation always