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

Move binary files to neoAssets #568

Merged

Conversation

AdamTadeusz
Copy link
Contributor

@AdamTadeusz AdamTadeusz commented Sep 8, 2024

Description

Move all asssets to neoAssets.

Toolchain

  • Windows MSVC VS2022
  • Linux GCC Distro Native [Specify distro + GCC version]
  • Linux GCC 10 Sniper 3.0

Linked Issues

@AdamTadeusz AdamTadeusz changed the title Copy binary files from neoAssets Move binary files to neoAssets Sep 8, 2024
@AdamTadeusz AdamTadeusz marked this pull request as draft September 8, 2024 19:06
@Rainyan
Copy link
Collaborator

Rainyan commented Sep 9, 2024

Do we want to pin this to a static GIT_TAG? That'd require PRs for both repos whenever there's an asset update on the binary side.

@AdamTadeusz
Copy link
Contributor Author

Do we want to pin this to a static GIT_TAG? That'd require PRs for both repos whenever there's an asset update on the binary side.

As this works right now you would have to do that anyway since the commit hash in the cmake file this repository has to be updated to use new assets in the asset repository. I probably am misunderstanding what you said though.

@AdamTadeusz
Copy link
Contributor Author

Do we want to pin this to a static GIT_TAG? That'd require PRs for both repos whenever there's an asset update on the binary side.

So you're just suggesting we use git tags instead of git commit hashes to reference versions of the assets repository? These and even branch names can be used if I understand correctly, unless we use use the git_shallow parameter in which case only branch names can be used I think

@AdamTadeusz
Copy link
Contributor Author

AdamTadeusz commented Sep 9, 2024

Never mind im retarded, you're suggesting we don't select the version of assets used. So yeah we could point to the master branch for example and add the git shallow tag and that would mean we don't need an extra pr here and it would be even quicker to fetch the git asset repository. The only question is would older versions of this repo break with newer assets. The only things that might break in the future are probably like materials, but also the stuff we are planning with navmeshes might for some reason not be compatible with older version of the repo I don't know

[Edit] Since the binary assets that are currently in this repository are version controller it seems like a safe bet to continue that practice by grabbing a specific version of those assets once they're in a different repository, but again I think it's unlikely anything will break with newer assets

@Rainyan
Copy link
Collaborator

Rainyan commented Sep 9, 2024

Yeah, I meant we most likely want to use a tag that points to the most recent change (like master), to avoid having to create somewhat superfluous PRs in the code repo to manually keep the two in sync whenever the binary repo receives an update.

Pinning dependencies by hash is nice for reproducability, but for convenience we probably don't need super strict control over the asset versioning for the assets in particular. At least not for dev builds.

@AdamTadeusz
Copy link
Contributor Author

Ill mark this as ready for review once Masterkatze has time to mess around with copy_directory versus the install command

@AdamTadeusz AdamTadeusz marked this pull request as ready for review September 10, 2024 16:29
@Masterkatze
Copy link
Contributor

LGTM.

The use of copy_directory is fine since we copy it inside the same repo.

If someone wants to use manually cloned neoAssets:

  1. Set FETCHCONTENT_FULLY_DISCONNECTED to ON
  2. Set FETCHCONTENT_SOURCE_DIR_NEO_ASSETS to the path of cloned neoAssets repo

@AdamTadeusz AdamTadeusz requested a review from a team September 20, 2024 11:12
@AdamTadeusz AdamTadeusz merged commit e6f1020 into NeotokyoRebuild:master Sep 20, 2024
6 checks passed
@nullsystem nullsystem added this to the v8.2-prealpha milestone Sep 25, 2024
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.

5 participants