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

Implement system to handle varying platform module names/compatibility across platforms #151

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AndrewKahr
Copy link
Member

@AndrewKahr AndrewKahr commented Feb 1, 2022

This serves as an alternative to #150 and game-ci/versioning-backend#39. It adds an additional internal github action to handle issues caused by varying module names and compatibility being added for additional target platforms by Unity. It can mark target platforms as incompatible so the system skips building it, for example, GameCI doesn't provide Android on linux below 2019.3 so the action is set such that Android is incompatible on Linux for any version below 2019.3. This also handles modules being renamed over time. For example, uwp-il2cpp became universal-windows-platform in 2019. The action has a mapping defined such that it knows when universal-windows-platform is requested, and the version of unity is below 2019, it will return the correct module name of uwp-il2cpp for the docker build while it remains universal-windows-platform in GameCI labels to reduce complexity in Unity-Builder. Since there is no clean divide between legacy and postX versions, there would need to be work done in versioning-backend every time we needed a new workflow, and more complexity added when windows and linux compatibility diverge. Having this compatibility action should make it easier to handle new modules being added in later versions without needing to make adjustments to the versioning-backend. Unlike #150 and game-ci/versioning-backend#39, this requires no changes on the versioning-backend for functionality.

Changes

  • Add new internal GitHub Action module-compatibility-handler
  • Make windows editor workflows check the module-compatibility-handler for module naming and whether a specific module is compatible. (The action is also compatible with Linux, we can add support into the workflows later if deemed necessary)
  • Build il2cpp for all versions (This was mistakenly disabled for legacy versions)
  • Ported report-to-backend to typescript and stopped tracking node modules

Checklist

  • Read the contribution guide and accept the code of conduct
  • Readme (updated or not needed)

Copilot summary

This pull request introduces a new GitHub Action named "Module Compatibility Handler" and includes several configuration and source code changes to support it. The most important changes include adding configuration files for code formatting and linting, defining the action's behavior, and implementing the core logic for module compatibility checks.

Configuration and Setup:

Core Logic Implementation:

@github-actions
Copy link

github-actions bot commented Feb 1, 2022

Cat Gif

@@ -0,0 +1,24 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

While reviewing, I asked myself this question:

Do we have to track node_modules folders in workflows? I'm not 100% sure of the answer, but we're already doing it here:
https://github.com/game-ci/docker/tree/main/.github/workflows/actions/report-to-backend

👍

Copy link
Member

Choose a reason for hiding this comment

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

We should in fact not do this, even if we are already. It's better to transpile so you can treeshake dependencies.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've converted report-to-backend and module-compatibility-handler to typescript and untracked the node modules. I'm not sure if the configs are set up correctly, I just mimicked what I saw in unity-builder. I also don't know how to set up the precommit hooks with multiple ts projects within a subdirectory of the main repo. If I could get some pointers that would be very helpful.

* Returns 1 if versionA is greater than versionB
* Returns 0 if the versions are identical
*/
function compareVersions(versionA, versionB) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if there's an easier way for version comparison. Also I feel like this could break for beta versions if we want to support that for the future (but hey I didn't test this at all)

Copy link
Member Author

Choose a reason for hiding this comment

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

I just added support for looking at the version letter and the number after it. I'm assuming a comes before b which comes before f. Then finally compare by the number after the letter. Let me know if there is anything I am missing there as I am not too well versed with Unity's versioning scheme.

@AndrewKahr AndrewKahr marked this pull request as draft January 27, 2023 01:33
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.

3 participants