ProjectBorealis / UEGitPlugin

Unreal Engine Git Source Control Plugin (refactored)
MIT License
494 stars 104 forks source link

lockonly breaks checkout prompt #99

Open Schroedingers-Cat opened 1 year ago

Schroedingers-Cat commented 1 year ago

I'm experimenting with a non-LFS approach for storing large binaries, using the LFS system only for file locking. Most of the problems solved by storing large binaries with LFS can also be solved by using the built-in partial clone feature.

Therefore, I modified the recommended .gitattribues like this:

[attr]lock filter=lfs diff=lfs merge=binary -text lockable
[attr]lockonly lockable
[attr]lfs filter=lfs diff=lfs merge=binary -text
[attr]lfstext filter=lfs diff=lfstext merge=lfstext -text
# Unreal Engine file types.
*.uasset lockonly
*.umap lockonly

The only problem I encountered with this is that the git plugin will ignore the bPromptForCheckoutOnAssetModification=True setting from Config\DefaultEditorPerProjectUserSettings. The prompt for checking out an existing file on modification doesn't appear.

Nevertheless, files can still be manually locked/checked out and the plugin will automatically release the locks when submitting the files.

Once a file has been manually locked from the command line, the prompt for checking out an existing file on modification will appear, also for subsequent changes after submitting.

Any idea why the checkout prompt needs to "poked" via a manual LFS lock first?

mastercoms commented 1 year ago

Thanks for the report, I'll look into why this is happening.

As a side note, keep in mind there are 2 problems with large files in git, one is a problem with per file scale, and the repo size itself. Partial clone only solves the latter. Git has trouble with its binary diff algorithm in many cases and even on a few binary files of a few megabytes changed, will fail to transfer to/from the remote. This is something we have tested several times in the past. Storing binary content (using LFS) outside of the Git repo was the best option each time.

Schroedingers-Cat commented 1 year ago

Thanks a lot for sharing your experience with this. Regarding the transfer problem with the diff algorithm, did disabling the delta compression for those binary files like *.uasset binary -delta improve the situation?

mastercoms commented 1 year ago

We didn't consider that since we would have full history for files, and it would bloat the repository quickly. I think partial cloned have improved the situation for clients somewhat since then. But we also discovered issues with transferring big files on first push/pull over Git HTTP on GitLab and Azure, maybe others (timeouts, remote ended connection, etc.), not sure if those providers have improved. It's a moot point on GitHub as we use now though since they block large files from being uploaded.

mastercoms commented 1 year ago

What is the output on git check-attr lockable -- *.uasset *.umap

Schroedingers-Cat commented 1 year ago

What is the output on git check-attr lockable -- *.uasset *.umap

$ git check-attr lockable -- *.uasset *.umap
*.uasset: lockable: set
*.umap: lockable: set

But we also discovered issues with transferring big files on first push/pull over Git HTTP on GitLab and Azure, maybe others (timeouts, remote ended connection, etc.), not sure if those providers have improved.

We had similar problems years ago until there was LFS. I'll try to test this one specifically, thanks for bringing it up!

mastercoms commented 1 year ago

Could you retest for this issue with the latest release?

Schroedingers-Cat commented 1 year ago

Thanks a lot for improving this!

Did some quick tests and overall the checkout dialog opens up when hitting ctrl+s. However, it seems that for some actors the checkout dialog will never appear even though there's the engine notification that these actors need check out. Also testing with git lfs locks shows that these actors are missing a lock. I'm not seeing a pattern to why a certain actor doesn't get the checkout dialog.

This should be easily reproducible as I'm just creating a first person template with v3.12 of the plugin + the gitattributes file from above and duplicating around 30 of these "SM_ChamferCube" actors in the FirstPersonMap.