BepInEx / BepInEx.ConfigurationManager

Plugin configuration manager for BepInEx
https://www.patreon.com/ManlyMarco
GNU Lesser General Public License v3.0
231 stars 53 forks source link

[WIP] Add background canvas to block clicks behind UI #11

Closed benediktwerner closed 3 years ago

benediktwerner commented 3 years ago

The Unity UI already blocks clicks for the most part but it doesn't work when the game is using Rewired for its input. In that case, clicking on the settings window also produces a click in the game and for example, can move a character in the background.

This PR adds a canvas behind the settings window that catches all clicks. I'm not sure if that's the best solution but it's what UnityModManager uses and it seems to work well.

The only issue left to solve is that this introduces a dependency on UnityEngine.UI (which is not yet added by the PR). I'm not sure if there are any issues regarding backward compatibility in that regard and also what the usual way to add the dependency would be. I guess you probably know more about that. I assume it would require adding a stripped version of the dll?

ManlyMarco commented 3 years ago

Can you give an example of a game where this is an issue, and steps to reproduce?

benediktwerner commented 3 years ago

The main game I'm interested in and where it's an issue is Desperados III but not sure if that's a good game to test since it's pretty big. The only other game using Rewired that I have installed is "Golf Peaks" which is pretty small. It's not really an issue there but you can still see it happening from the fact that the game's UI hover effects change when moving the mouse above the config menu. But clicks are actually blocked here as by the way also in Desperados when in the main menu, probably because it's a different method that is used there. But inside a level in Desperados, it completely lets clicks through and constantly gives move commands to the characters.

ManlyMarco commented 3 years ago

Related https://github.com/ManlyMarco/RuntimeUnityEditor/issues/17

benediktwerner commented 3 years ago

Ok, as mentioned in the other issue I solved that with a separate plugin now but I don't think that's the best or even really a possible solution here. To be honest, the mouse going through is not such a major problem but IMO it should still be fixed, especially since the solution doesn't seem very problematic to me.

It's also not really a Rewired specific solution and might also be beneficial for some other input managers that similarly do their own mouse stuff.

If the requirement on UnityEngine.UI is really an issue that should be easy to work around with a check on startup that otherwise just falls back to the old behavior and since the method is called rarely there also shouldn't be any issues with performance when using reflection or anything like that.

P.S.: Feel free to ping me on the BepInEx Discord as 1vader#0203 if you prefer to discuss it over there.

ManlyMarco commented 3 years ago

It's also not really a Rewired specific solution and might also be beneficial for some other input managers that similarly do their own mouse stuff.

For now this is the only case that we know of this happening, so it's effectively a Rewired-specific issue. If there are more cases of this happening then we will think about handling this, until then a specialized plugin is the way to go.

benediktwerner commented 3 years ago

Ok, understandable, I guess the DisplayWindowChanged event did indeed make this not too hard. I now added this to my plugin as well.