spoiledcat / git-for-unity

MIT License
469 stars 30 forks source link

Auto lock support #3

Open SamuHell opened 1 year ago

SamuHell commented 1 year ago

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

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 commented 1 year ago

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

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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?

buoutuanzi commented 8 months ago

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

STARasGAMES commented 6 months ago

+1 Asset locking is very much appreciated