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

Rewrite asset detection + add many more assets to our detection. #462

Merged
merged 14 commits into from
Jul 29, 2024

Conversation

Beyley
Copy link
Member

@Beyley Beyley commented May 6, 2024

This adds all missing assets from toolkit (thanks Aidan!)

We also now store the serialization method (binary, compressed texture, text, etc), and the "Asset Safety Level" differently.

Asset Safety Level has been replaced with a new AssetFlags flags enum, which stores whether a specific asset is "Dangerous", "Media", and/or "Modded".

This allows us to much more gradually block assets, e.g. block only Media assets, but allow other types of Modded assets, which was not possible before.

Opening as draft as i have yet to have the time to fully test this ingame, but tests pass locally

May fix #438 aswell, but that needs to be tested

This adds all missing assets from toolkit (thanks Aidan!)

We also now store the serialization method (binary, compressed texture, text, etc), and the "Asset Safety Level" differently.

Asset Safety Level has been replaced with a new AssetFlags flags enum, which stores whether a specific asset is "Dangerous", "Media", and/or "Modded".

This allows us to much more gradually block assets, e.g. block only Media assets, but allow other types of Modded assets, which was not possible before.
@Beyley Beyley requested a review from jvyden May 6, 2024 06:39
Copy link
Member

@jvyden jvyden left a comment

Choose a reason for hiding this comment

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

Feeling a bit mixed on the implementation of this.

Refresh.GameServer/Configuration/GameServerConfig.cs Outdated Show resolved Hide resolved
Refresh.GameServer/Importing/AssetImporter.cs Outdated Show resolved Hide resolved
Refresh.GameServer/Importing/Importer.cs Outdated Show resolved Hide resolved
Refresh.GameServer/Types/Assets/AssetFlags.cs Show resolved Hide resolved
Refresh.GameServer/Types/Assets/GameAssetType.cs Outdated Show resolved Hide resolved
Refresh.GameServer/Types/Assets/GameAssetType.cs Outdated Show resolved Hide resolved
Refresh.GameServer/Types/Assets/GameSerializationMethod.cs Outdated Show resolved Hide resolved
@jvyden
Copy link
Member

jvyden commented May 12, 2024

Conflicting with master.

Also, are there any unit tests that should be added to help with this/asset detection in general? We have a few asset tests already but I'm wondering if we can do more.

# Conflicts:
#	Refresh.GameServer/Configuration/GameServerConfig.cs
#	Refresh.GameServer/Database/GameDatabaseProvider.cs
#	Refresh.GameServer/Endpoints/ApiV3/InstanceApiEndpoints.cs
@Beyley
Copy link
Member Author

Beyley commented May 19, 2024

are there any unit tests that should be added to help with this/asset detection in general? We have a few asset tests already but I'm wondering if we can do more.

ideally we have a set of test assets stored in tree which test our dependency tree reading, asset type, serialization method, etc

Beyley and others added 5 commits May 18, 2024 18:22
Co-authored-by: jvyden <jvyden@jvyden.xyz>
Signed-off-by: Beyley Thomas <ep1cm1n10n123@gmail.com>
Co-authored-by: jvyden <jvyden@jvyden.xyz>
Signed-off-by: Beyley Thomas <ep1cm1n10n123@gmail.com>
@Beyley
Copy link
Member Author

Beyley commented May 21, 2024

No tests yet, but i resolved all the previous review comments

Beyley and others added 3 commits May 25, 2024 03:17
Co-authored-by: jvyden <jvyden@jvyden.xyz>
Signed-off-by: Beyley Thomas <ep1cm1n10n123@gmail.com>
@Beyley
Copy link
Member Author

Beyley commented May 30, 2024

still needs asset tests, but rebased to latest master and ready for more review

@jvyden
Copy link
Member

jvyden commented Jun 4, 2024

Looks fine but I probably won't review again until we have good tests for this.

Beyley and others added 2 commits June 4, 2024 14:24
Co-authored-by: jvyden <jvyden@jvyden.xyz>
Signed-off-by: Beyley Thomas <ep1cm1n10n123@gmail.com>
@jvyden
Copy link
Member

jvyden commented Jul 29, 2024

This is bitrotting to hell and back so I'm just going to re-review, test it manually by re-importing all the assets locally and merge as-is if successful.

@jvyden jvyden marked this pull request as ready for review July 29, 2024 02:22
@jvyden jvyden self-requested a review July 29, 2024 02:22
@jvyden
Copy link
Member

jvyden commented Jul 29, 2024

Migration seems to be okay

@jvyden jvyden merged commit 7d0a88a into LittleBigRefresh:main Jul 29, 2024
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.

Allow LBP3 Quests
2 participants