Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove CommonAPI dependency #612

Merged
merged 3 commits into from
Dec 14, 2023
Merged

Conversation

starfi5h
Copy link
Collaborator

  • Replace CommonAPI functions so no need to install CommonAPI in the future.
  • Add CreateHotkeyControl to support KeyboardShortcut type in MultiplayerOptions.
    The available Keycode string can be found in Unity Doc. The input text will turn red if it is invalid.
  • Add ChatHotkey, ChatWindowOpacity config options

chat config

@PhantomGamers
Copy link
Collaborator

Why is CommonAPI still included as a soft dependency then? Do we leverage it at all after this PR?

@starfi5h
Copy link
Collaborator Author

The soft dependency is added to preserve the load order that Nebula is loaded after CommonAPI. I'm not sure if CommonAPI has code that is sensitive to load order, so maybe it can be removed?

As for usage, here are some warnings from chatwindow after disable CommonAPI

[Warning: Unity Log] The referenced script (CommonAPI.UIWindowResize) on this Behaviour is missing!
[Warning: Unity Log] The referenced script (CommonAPI.MaterialFixer) on this Behaviour is missing!
[Warning: Unity Log] The referenced script on this Behaviour (Game Object 'ChatV2') is missing!
  • the resize window feature has to use CommonAPI to enable. (Or maybe add the component in this PR?)
  • MaterialFixer is to fix TranslucentImage, so the component is removed in the PR.
  • I'm not sure the effect of the last one. Haven't tested it out.

So after this PR, Nebula can run without CommonAPI but with less features. Whether to include CommonAPI in thunderstore dependencies can be discussed.

Copy link
Collaborator

@PhantomGamers PhantomGamers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine to me for now, at least to make developing the update easier on us, but before release we should update the chat assetbundle to remove the dependency on CommonAPI then as well.

@PhantomGamers PhantomGamers merged commit 01ed3fb into NebulaModTeam:master Dec 14, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants