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

Auto lock support #3

Open
SamuHell opened this issue Oct 19, 2022 · 7 comments
Open

Auto lock support #3

SamuHell opened this issue Oct 19, 2022 · 7 comments
Labels
feature A new feature proposal and/or discussion

Comments

@SamuHell
Copy link

Hello! Just tested real quick with this updated version with 2021LTS. It's great this project has been packagified and has been ported to latest LTS! Thank you!
Dunno where you're at with porting over issues from the old github, but here's one:

This is mostly a feature request: right now when editing a scene, I'll still be able to edit it even though the scene is unlocked. There's no prompt to ask me to lock a scene.
Same when someone else has locked a scene already, at that point I shouldn't be able to edit it at all.
The above would be the main reason for me to use this package, as locking + unity can't be achieved outside of a unity plugin (same as the perforce plugin that's required to use unity).

Thanks a bunch!

Prerequisites

  • Be sure to run with tracing enabled to capture runtime details in the log file
  • Include the log file in the PR.
    • On Windows, the extension log file is at %LOCALAPPDATA%\GitHubUnity\github-unity.log
    • On macOS, the extension log file is at ~/Library/Logs/GitHubUnity/github-unity.log
    • On linux, the extension log file is at ~/.local/share/GitHubUnity/github-unity.log

Description

Steps to Reproduce

  1. [First Step]
  2. [Second Step]
  3. [and so on...]

Expected behavior: [What you expect to happen]

Actual behavior: [What actually happens]

Reproduces how often: [What percentage of the time does it reproduce?]

Additional Information

Any additional information, configuration or data that might be necessary to reproduce the issue.

@shana
Copy link
Member

shana commented Oct 19, 2022

There's APIs that could make this possible, but git lfs is very, hmmm, convoluted when it comes to locking apis, it requires an actual server user, which Git for Unity doesn't have, so this requires a bit more work than just hooking up to IsOpenForEdit and popping up a dialog.

I would say this would need

  • configuration backend and UI for setting user information specifically for lfs
  • hooking up to Unity edit apis
  • UI for showing lock status and/or allowing requesting locks on edit, denying edits when locks are taken, showing who has a lock

Some of this was done in GitHub for Unity, but the GitHub-specific auth backend was removed in the fork, and GitHub deprecated all the auth apis, so none of that works anymore. And, of course, none of this should be GitHub-specific, so the user configuration for lfs needs to be done in a way that would work for any lfs server.

lfs sucks, and lfs locking extra sucks, and while the use case for locking is very valid, I don't think lfs should be handling locking at all - it doesn't do it well, and it does it too late, and why is a storage service handling editing permissions in the first place anyway? It's a storage server, it stores things, it shouldn't be responsible for whether you can edit your scene or not.

I'll think about it.

@SamuHell
Copy link
Author

yeah, agreed it's quite frankenstein.
You need a centralized, atomic, source of information for locking, else you'll end up with conflicts on who locked the file in the end. git by it's distributed nature isn't great for that. Perforce having a centralized server for this makes sense. Git, you need the centralized aspect to do that. But yeah, it sucks :P
I believe no matter what git repo provider users use, there will always be the provider that doesn't provide locking. Bitbucket integrated locking support years later than Github for example. If users have custom repo solutions that support vanilla git, there will always be a case without locking support. Locking in the plugin would have to be enabled but only if available.
Plus what are the major providers? Gitlab, Github, Bitbucket? There could maybe be a way to not have to handle all the others at once and just provide an extensible solution where users that want to use an unsupported provider can just extend it and even submit a PR for it?

@shana
Copy link
Member

shana commented Oct 20, 2022

Git providers shouldn't be providing locking - like lfs, git is a storage database, whatever it does is way too late when it comes to you wanting to edit a scene. Every single provider is going to implement a completely different system, with completely different features and separate user matching implementations, and chasing that is the worst. lfs locking doesn't support per branch or per group of branches locking, which means if you request a lock on a feature branch, nobody working on a development branch will be able to edit something - it kills whatever usefulness locking has in lfs, so you can throw that out of the window as well. No, this needs a completely separate permissions service that can track the information of who wants the lock in which scope, a service that only does that one thing, independent of whatever git or large file storage you happen to be using to store your stuff.

Also, nitpick here, but git is not a distributed system in the reality of 99% of teams - you have a git server where you store your stuff, that server is authoritative in the large majority of cases, nobody is going to be going around juggling multiple different git server remotes to share their stuff with other people. You push to the server, you pull from the server, it's centralized as far as you're concerned. What's really important to keep in mind is that it's not a "server" with features that you can configure, it's just a dumb database running somewhere that you have no control over. Trying to tack on features like locking on top of it is really a case of round hole, square peg (or the other way around, I can never remember). That's not to say it's useless - we can certainly run other services to provide extra features for version control workflows - it's just that it's a database, that's all it is, it tracks no state beyond what you store in it.

@SamuHell
Copy link
Author

lfs locking doesn't support per branch or per group of branches locking, which means if you request a lock on a feature branch, nobody working on a development branch will be able to edit something - it kills whatever usefulness locking has in lfs, so you can throw that out of the window as well.

I'd say this is a feature actually. The goal of a locking mechanism is to prevent merge conflicts on unmergeable assets like images and the such. If I work on a feature branch and want to merge to develop while someone else already changed the asset on develop, that'll create a merge conflict I can't resolve when trying to merge my feature branch back to develop. A whole-repo locking mechanism would help with this.

Perforce has per-stream locking cause their workflow involves way less branches than git. They added branches quite recently and most of the main workflows are branch less. When I was working on productions that were using perforce, yes there was always the odd "hey! the intern forgot to unlock asset x before going on vacation, no one can edit it!" but there was always a tech lead able to force unlock the thing. Plus this forces good practices on asset separation (for example making sure your scenes are separated correctly with domain based additive scene loading; lighting artists would work in their own scene, audio same, level designers same, etc). This way, you reduce the surface for merge conflicts in the first place, locking becomes an edge case safeguard, not something you bump your head against all the time.

What other locking provider exists that'd be easy to use by users and that's in sync with the whole repo?

@SamuHell
Copy link
Author

Plus thinking of it, the locking mechanism would need to be aware of git logic, no? Take this case:
You have branch "develop"
Create branch "feature1" branched off develop and create fileA
Create branch "feature1-1" branched off "feature1" and try to update fileA
If we had a separate locking mechanism tracking develop, it wouldn't be aware of fileA at all, since it only exists on branch feature1? I'm not super familiar with git internals, would it be possible to track blobs across branches, something like that?

@CapnRat CapnRat added the feature A new feature proposal and/or discussion label Nov 8, 2022
@buoutuanzi
Copy link

Will these functions be added? I also downloaded this plug-in for these functions, but found that it was not implemented.

@STARasGAMES
Copy link

+1
Asset locking is very much appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature proposal and/or discussion
Projects
None yet
Development

No branches or pull requests

5 participants