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 basic mod lints #55

Merged
merged 5 commits into from
Aug 15, 2023
Merged

Implement basic mod lints #55

merged 5 commits into from
Aug 15, 2023

Conversation

jieyouxu
Copy link
Collaborator

@jieyouxu jieyouxu commented Aug 13, 2023

This PR adds a new cli action ActionLint, run as drg_mod_integration lint $PROFILE, to perform some basic mod lints for the given $PROFILE. This PR also adds a mod lint button and a lint report GUI.

Screenshot 2023-08-14 141837

Currently implemented mod lints:

  • Lint mod conflicts.
  • Lint AssetRegistry.bin.
  • Lint incorrectly included shader files.
  • Lint outdated pak version.
  • Lint empty archive.
  • Lint archive containing only non-pak files.
  • Lint archive containing multiple paks.

Some basic manual testing can be done by creating a "test" profile which consists of two mods:

  1. https://mod.io/g/drg/m/cave-leech-obi-wan-hello-there#1937463
  2. https://mod.io/g/drg/m/spy-leech#2040409 (this has shader bytecode)

Pending Work

  • Need to add some integration tests for the basic lints. We'd probably want to have some test mods for testing mod integration as well anyway.

cc #39.

@jieyouxu jieyouxu marked this pull request as draft August 13, 2023 03:33
@jieyouxu jieyouxu marked this pull request as ready for review August 13, 2023 06:37
@jieyouxu jieyouxu force-pushed the mod-lint branch 6 times, most recently from 8e19373 to 2f08dac Compare August 14, 2023 06:25
@trumank
Copy link
Owner

trumank commented Aug 14, 2023

It's not clear to me what test_mod_batches is supposed to me. I would opt for test_assets to contain anything test-related that's not actual code.

I'm also considering not including any archives (zip or pak) in the repo at all and instead build them from loose files when needed. This would make modification easier and would also provide more meaningful diffs. However, any tests are better than no tests so I'm find with how it is currently. Something to think about for future improvements.

@jieyouxu
Copy link
Collaborator Author

jieyouxu commented Aug 14, 2023

It's not clear to me what test_mod_batches is supposed to me. I would opt for test_assets to contain anything test-related that's not actual code.

Renamed to test_assets.

I'm also considering not including any archives (zip or pak) in the repo at all and instead build them from loose files when needed. This would make modification easier and would also provide more meaningful diffs.

I agree with this, though it would require the integration tests to have a build step and I'm not very sure how that would be nicely handled.

1. Lint mod conflicts
2. Lint AssetRegistry.bin
3. Lint incorrectly included shader files.
4. Lint outdated pak version
5. Lint empty archive.
6. Lint archive containing only non-pak files.
7. Lint archive containing multiple paks.
@trumank trumank merged commit 6813c26 into master Aug 15, 2023
3 checks passed
@trumank trumank deleted the mod-lint branch August 15, 2023 01:57
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.

None yet

2 participants