PlayEveryWare / eos_plugin_for_unity

Repository for PlayEveryWare's EOS Plugin for Unity, bringing the functionality of Epic Online Services to the Unity Game Engine.
https://eospluginforunity.playeveryware.com
288 stars 56 forks source link

NewInputSystem is forced to be installed, which is not desirable. #402

Closed mohumohu-corp closed 1 year ago

mohumohu-corp commented 1 year ago

Hi,

First of all, thank you for continually updating this project.

Starting with the latest version 3.0.0, the NewInputSystem is now forced to be installed. This will cause projects using the old InputSystem to receive the following warning every time the project is started

Many people prefer to use the old InputSystem because the new InputSystem has latency issues and many unnecessary features. I am one of them.

Using the AssemblyDefinition's VersionDefines feature, you can write code that will only work for projects that have the NewInputSystem installed. That way, it will not affect projects that use the old InputSystem.

I have not read all the source code in this project, but I would like to see this problem fixed. captured

paulhazen commented 1 year ago

Hmmm.... I wonder what the best behavior here is. It seems like a good idea to default to the newest input system, but it should be made easy to prefer to use the old input system.

One solution is to remove the following define in Assets/Plugins/Source/Editor/com.playeveryware.eos-Editor.asmdef:

image

But that feels like a non-intuitive hassle.

Regardless, does removing that define fix the problem for you @mohumohu-corp?

Failing that, do you have a suggestion for making this smoother without defaulting to the old input system (or is the new input system so bad that nobody is likely to be using it anytime soon in your opinion)?

mohumohu-corp commented 1 year ago

I have not read the source code of this update, so I cannot say for sure.

But I think it would be better to disable the automatic installation and enclose only the code parts for NewInputSystem in COM_UNITY_MODULE_INPUTSYSTEM directives. (So, yes, removing that definition would solve the problem.)

Some mobile developers prefer the old input system because the NewInputSystem consumes a lot of CPU time just to have it installed.

To change the assembly definition settings, this package would have to be used in "Embed "mode, which I don't want to do.

Since the core part of this package should not depend on NewInputSystem, why not enclose only the dependent parts with the COM_UNITY_MODULE_INPUTSYSTEM directive?

But again, I appreciate the effort you are putting into updating this package.

paulhazen commented 1 year ago

Since the core part of this package should not depend on NewInputSystem, why not enclose only the dependent parts with the COM_UNITY_MODULE_INPUTSYSTEM directive?

This does seem like the appropriate step to take. Give me the day to look into it and I'll have an answer or PR for you by EOD.

paulhazen commented 1 year ago

@mohumohu-corp Apologies for not getting back to you by EOD like I said I would. It'll be the first thing I do this coming Monday morning.

paulhazen commented 1 year ago

@mohumohu-corp

Using the AssemblyDefinition's VersionDefines feature, you can write code that will only work for projects that have the NewInputSystem installed. That way, it will not affect projects that use the old InputSystem.

This seems like a cogent way to handle the problem - would you be kind enough to point me toward the documentation for this?

paulhazen commented 1 year ago

This issue is resolved now in both the stable and development branches.