razerofficial / CChromaEditor

C++ Dynamic library for playing and editing Chroma animations
MIT License
39 stars 14 forks source link

SetIdleAnimation does nothing #8

Open BenHoffmanEpic opened 5 months ago

BenHoffmanEpic commented 5 months ago

Hi there! I am seeing an issue where the PluginSetIdleAnimation function just does nothing.

  1. Load an animation (i am using PluginOpenAnimationFromMemory)
  2. set that animation as the idle animatoin
  3. nothing happens

Reading the docs, I would expect that the animation starts playing when nothing else is playing.

I have also done:

  1. Load animation A
  2. Load animation B
  3. Set animation A as the idle anim
  4. Play animation B
  5. When animation b finishes, I would expect Anim A to start playing as the "idle" animation, but it does not.

Any feedback around this would be appreciated! thanks

tgraupmann commented 5 months ago

It's entirely possible you are on an old version of the plugin. This could be an issue I fixed. Try sample effect 48 which demonstrates how to use the idle animation, play an animation, and then fallback to the idle animation.

https://github.com/razerofficial/CSDK_SampleApp/blob/SUPPORT_UNICODE/Sample.cpp#L4730

I may need to add sample 48 to the Unreal plugin and then I'll share that sample code as well. When I add samples to the core C++ sample it has to propagate out to the other plugins. The API is mirrored across plugins so it would be nearly the same methods in every plugin.

BenHoffmanEpic commented 5 months ago

Hey there, I am on version 1.0.0.9, and reading the release notes for future versions I dont see any info about a bug fix for the idle animation stuff. Unless that "Improve idle logic" is what you are referring to?

Unreal Engine has our integration of the razer chroma officially targeting UE 5.5 . You can view it on gitHub here

tgraupmann commented 5 months ago

You can find the update Unreal update here.

I ported Effect48 to the Unreal plugin so you can see a working idle implementation.

https://github.com/razerofficial/Unreal_ChromaSDK/blob/SUPPORT_UNICODE/Chroma_Sample/Source/Chroma_Sample/SampleAppChromaBP.cpp#L5098

I haven't tested UE 5.5 specifically, but all you need to do is edit the uplugin EngineVersion to:

"EngineVersion": "5.5.0",

https://github.com/razerofficial/Unreal_ChromaSDK/blob/SUPPORT_UNICODE/Chroma_Sample/Plugins/ChromaSDKPlugin/ChromaSDKPlugin.uplugin#L9

tgraupmann commented 5 months ago

Hey there, I am on version 1.0.0.9, and reading the release notes for future versions I dont see any info about a bug fix for the idle animation stuff. Unless that "Improve idle logic" is what you are referring to?

Unreal Engine has our integration of the razer chroma officially targeting UE 5.5 . You can view it on gitHub here

@BenHoffmanEpic

I was not aware of this project. Nice! I recognize a lot of the code based on an older version of the plugin. It just needs a few things merged up to 1.0.1.1 so you can get the idle fixes and updated Chromatic.DLL support with the Chroma Sensa haptics APIs.

BenHoffmanEpic commented 5 months ago

Yea we are shooting for just the basic lighting functionality provided by Chroma to play Chroma animation assets for our official engine support of it. I will test out upgrading to 1.0.1.1 locally and see if that resolves our idle animation issues for us.

tgraupmann commented 5 months ago

Yea we are shooting for just the basic lighting functionality provided by Chroma to play Chroma animation assets for our official engine support of it. I will test out upgrading to 1.0.1.1 locally and see if that resolves our idle animation issues for us.

@BenHoffmanEpic

I see the methods that you have exposed.

https://github.com/EpicGames/UnrealEngine/blob/ue5-main/Engine/Plugins/Experimental/RazerChromaDevices/Source/RazerChromaDevices/Public/RazerChromaFunctionLibrary.h

Please include a couple important functions which should still keep the implementation small.

    /*
    Name the Chroma event to add extras like haptics to supplement the event
    */
    UFUNCTION(BlueprintCallable, meta = (DisplayName = "SetEventName", Keywords = "Name Chroma events to add extras"), Category = "ChromaSDK")
    static int32 SetEventName(const FString& name);
    /*
    On by default, `UseForwardChromaEvents` sends the animation name to `CoreSetEventName`
    automatically when `PlayAnimationName` is called.
    */
    UFUNCTION(BlueprintCallable, meta = (DisplayName = "UseForwardChromaEvents", Keywords = "Automatically forward animation names as Chroma event names"), Category = "ChromaSDK")
    static void UseForwardChromaEvents(bool toggle);
tgraupmann commented 5 months ago

@BenHoffmanEpic

Is it possible to expose this experimental plugin to Verse? That would allow UEFN mods to play Chroma Animations if they are able to add Chroma Animations as content.

BenHoffmanEpic commented 4 months ago

this feature is only in beta right now, we will not be adding it to UEFN any time soon until we can properly test everything.

Also, if you have methods that you want to be added which are not currently a part of our use cases, then feel free to make a pull request to the engine on github for it.

BenHoffmanEpic commented 4 months ago

So updating to version 1.0.1.1 seems to have partially solved the issue.

It will work when being called once, but it is unable to be changed at runtime. For example, if I :

  1. Call Set Idle animation with "Anim 1"
  2. Play some different animation ("Anim 2")
  3. Call set idle animation to something different then it was ("Anim 3" )

When "Anim 2" finishes, then "Anim 1" is still playing because it is still the current idle animation. "Anim 3" never gets set as the idle animation.

This makes it pretty difficult for designers to easily use these effects. Here's an example in the context of a Fortnite BR match:

  1. You start the match, we set a "walking" idle animation that we want to play during normal gameplay.
  2. You pick up a shield potion or something, and start using it.
  3. While you are using it, the storm comes in and you are now inside the storm.
  4. We want to change the idle animation to now the be "Storm" animation, instead of the "walking" idle animation, but we cant.

Something that could help alleviate this would be the ability to access the length (in seconds) of each animation being played. That way we can properly support a "play one shoot" style of effect, which would integrate very nicely into existing engine systems (not just in UE, but in other engines too). Is that possible?

Thanks Ben

tgraupmann commented 4 months ago

I was able to successfully test 5.5 with the updated plugin and added methods.

Commit: Upgraded to 1.0.1.1 - Added SetEventName and UseForwardChromaEvents

https://github.com/tgraupmann/UnrealEngine/commit/75f2a662e220233a80d4e590463bce524d6af75e

When the signature matches, the plugin loads.

https://github.com/tgraupmann/UnrealEngine/commit/8aad9d72e8ac50b5097021467ca844b03486c589

image

I'll expose the total duration in seconds in the DLL and blueprints and then I can send a PR.

Added PluginGetFrameDuration and PluginGetTotalDuration https://github.com/razerofficial/CChromaEditor/commit/b4a79c363e265f593be50c4eafa9ef6f6c924922

tgraupmann commented 4 months ago

I'll setup unit tests for that specific scenario of trying multiple idle animations. https://github.com/razerofficial/CChromaEditor/blob/SUPPORT_UNICODE/Cpp_UnitTests/UnitTest.cpp#L2222

I usually use a single animation and then manipulate that same animation. I either modify all the frames or copy another animation to the idle animation.

The Chroma game loop sample uses the API for dynamic mixing of multiple animations into a single animation. It uses animations or direct array access, but lacks using an idle animation. https://github.com/razerofficial/CSDK_ChromaGameLoopSample/tree/SUPPORT_UNICODE

As for getting the length of an animation, you can get the count for the number of frames and then you can get the duration of each frame to add up for the total duration. Through testing it's best to get the durations in the game loading phase and before playing animations that way you don't have to run the calculations numerous times. https://github.com/razerofficial/CChromaEditor/blob/SUPPORT_UNICODE/Cpp_UnitTests/UnitTest.cpp#L2286

I did find a bug that calling PluginGetFrame stops the animation. So that's another reason to save the animation duration before playing the animation.

    EXPORT_API int PluginGetFrame(int animationId, int frameIndex, float* duration, int* colors, int length, int* keys, int keysLength)
    {
        PluginStopAnimation(animationId);
...

The next version will not call StopAnimation when PluginGetFrame or PluginUpdateFrame are called. https://github.com/razerofficial/CChromaEditor/commit/2139e21aefd27741053ba544082742a6fcc06dac

tgraupmann commented 4 months ago

I was able to drag and drop Chroma animation files and import them to the content folder.

From the level blueprint I am able to play animations and I exposed Get Total Duration for an animation asset.

image

I need to create a signed build before I commit the DLL update into the branch.

Here's a temporary branch minus the DLL update.

Added GetTotalDuration

https://github.com/tgraupmann/UnrealEngine/commit/727b92e1edbc483336dabe652bd4ff9a1ed2a637

In the meantime since you don't have signature validation, you can build the latest CChromaEditorLibrary. https://github.com/razerofficial/CChromaEditor/tree/SUPPORT_UNICODE

And then copy it to:

UnrealEngine_UE5-Main\Engine\Plugins\Experimental\RazerChromaDevices\Binaries\ThirdParty\Win32
UnrealEngine_UE5-Main\Engine\Plugins\Experimental\RazerChromaDevices\Binaries\ThirdParty\Win64
tgraupmann commented 4 months ago

@BenHoffmanEpic

This adds the signed build for 1.0.1.2 with GetFrameDuration() and GetTotalDuration().

https://github.com/tgraupmann/UnrealEngine/commit/727b92e1edbc483336dabe652bd4ff9a1ed2a637

https://github.com/tgraupmann/UnrealEngine/commit/c701dbefcaa17d1b6d1434d0864ee3d5d4908a4b

I'll wrap this into a PR next after merging and testing the latest.

tgraupmann commented 4 months ago

@BenHoffmanEpic

I submitted the changes in a PR.

Upgraded CChromEditorLibrary to 1.0.1.2. Exposed library methods. #11983

https://github.com/EpicGames/UnrealEngine/pull/11983

tgraupmann commented 4 months ago

The Unicode version of the CChromaEditorLibrary adds support for Unicode characters in the animation name.

image

Support Unicode characters in the Chroma animation names

https://github.com/EpicGames/UnrealEngine/pull/11983/commits/089df52c07a8ed6d984a671c6b511e3d0d5432eb

tgraupmann commented 4 months ago

@BenHoffmanEpic

There's an issue with TitleBuilder.GetData() not being null terminated:

https://github.com/EpicGames/UnrealEngine/blob/ue5-main/Engine/Plugins/Experimental/RazerChromaDevices/Source/RazerChromaDevices/Private/RazerChromaDevicesModule.cpp#L103

FCString::Strncpy(AppInfo.Title, TitleBuilder.GetData(), 256);

image

The Synapse title shows garbled characters after the title.

Instead try:

FCString::Strncpy(AppInfo.Title, TitleBuilder.ToString(), 256);

image

This is a UE 5.5 repro project to reproduce the issue.

https://github.com/devinethang/UE_PuzzleGame

I submitted PR for the fix.

https://github.com/EpicGames/UnrealEngine/pull/12032

tgraupmann commented 3 months ago

The PR merges are complete.

https://github.com/EpicGames/UnrealEngine/commit/7710d6e91885e013c98d7600285318afa72708f7

You can see the ToString() fix here.

https://github.com/EpicGames/UnrealEngine/blob/ue5-main/Engine/Plugins/Experimental/RazerChromaDevices/Source/RazerChromaDevices/Private/RazerChromaDevicesModule.cpp#L103

tgraupmann commented 3 months ago

I tested and the ToString() fix works.

I found one packaging issue. If either file is missing, packaging development fails for Windows:

Engine\Plugins\Experimental\RazerChromaDevices\Binaries\ThirdParty\Win32\CChromaEditorLibrary.pdb

Engine\Plugins\Experimental\RazerChromaDevices\Binaries\ThirdParty\Win64\CChromaEditorLibrary64.pdb

BenHoffmanEpic commented 3 months ago

Hey there,

What scenario would one of those files be missing? You would have to manually delete the file from the engine, which there isn't much we can do about.

tgraupmann commented 3 months ago

Hey there,

What scenario would one of those files be missing? You would have to manually delete the file from the engine, which there isn't much we can do about.

I hadn't deleted the file. It just wasn't there with the default clone of the repository. I was doing the default development packaging when I discovered the issue.

image

The workaround is to copy the PDB to the expected place so packaging development can succeed.

image

The PBD symbols can be found here.

https://github.com/razerofficial/CChromaEditor/releases/tag/CChromaEditorLibrary-Windows-PC-Unicode-1.0.1.2

The debug symbols should be optional and typically not something required to package the build. You only really need them if you wanted to debug the Chroma editor library.

In my plugin, I only package the DLL using ChromaSDKPlugin.Build.cs.

https://github.com/razerofficial/Unreal_ChromaSDK/blob/SUPPORT_UNICODE/Chroma_Sample/Plugins/ChromaSDKPlugin/Source/ChromaSDKPlugin/ChromaSDKPlugin.Build.cs#L78

BenHoffmanEpic commented 3 months ago

Interesting, it seems like those binary files and PDP must be getting ignored on the UE Github page, i will look into it!

tgraupmann commented 1 month ago

Interesting, it seems like those binary files and PDP must be getting ignored on the UE Github page, i will look into it!

Any luck? I'm hoping the PDB issue gets resolved by the potential October 5.5 prerelease.

tgraupmann commented 3 weeks ago

Interesting, it seems like those binary files and PDP must be getting ignored on the UE Github page, i will look into it!

Any luck? I'm hoping the PDB issue gets resolved by the potential October 5.5 prerelease.

I can confirm that using the 5.5.0 version of Unreal Engine installed from the Epic Launcher works as expected in the editor and for packaged games.

I used this project to verify the process. https://github.com/devinethang/UE_PuzzleGame

image

BenHoffmanEpic commented 3 weeks ago

Heya, yes the PDB issue has been resolved for the binary and GitHub versions of the editor. Being that it's just a pdb file for debugging, we decided not to redistribute it with the editor and instead provide instructions for how to download it in a readme if users need it instead.

tgraupmann commented 3 weeks ago

Heya, yes the PDB issue has been resolved for the binary and GitHub versions of the editor. Being that it's just a pdb file for debugging, we decided not to redistribute it with the editor and instead provide instructions for how to download it in a readme if users need it instead.

The next step will be to enable the plugin in UEFN. If SetEventName("string_id") could be exposed from Blueprints to Verse that would be all that's needed from the API to control Chroma and haptics. The InitSDK call could be modified to use the UEFN mod information.

For testing features it looks like plugins can be activated in UEFN. https://www.youtube.com/watch?v=tgTBH9Ripc4

So the question would be how do you expose a BP Function to Verse from the plugin?

BenHoffmanEpic commented 3 weeks ago

Heya, getting things into UEFN/Verse is something that I would prefer not to chat about publicly, I would recommend reaching out to your contacts at Epic internally to discuss.

Thanks!