getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
214 stars 48 forks source link

Add CONVENTION.md #437

Open mottosso opened 4 years ago

mottosso commented 4 years ago

Related to #436

This PR contains a set of guidelines for writing graphical tools for Avalon. It's a starting point, along with an example program of what all guidelines look like together. It is meant as a reference for how to write tools that is..

  1. Consistent (with the world, but foremost within Avalon)
  2. Scalable (without hacks)
  3. Resilient to user error (i.e. handle exceptions, provide messages)
  4. Resilient to developer error (i.e. work when parts are broken, "hop on one leg")

The example implements some of the guidelines at the moment, with the idea of being both textual and codified reference of the guidelines (it's a "guideline browser").

It's currently missing:

  1. Multiple models, why, and how to deal with that
  2. Asynchronousity, threading
  3. Reference to which guidelines are implemented where (e.g. E3, S5 etc.)
  4. External resources, like images (I noticed Avalon does not yet use any!)
  5. CSS
python -m avalon.tools.convention \path\to\avalon-core\CONVENTION.md

image

More examples include..

Except these guidelines are a superset of what those were built with, an improvement.

As a guideline to the guidelines, let's refer to..

For #436, I'd like for these guidelines to apply, but they're not perfect. What I'd like you to do next is have a look at the example program and see how you can improve it. It'll be the baseline for any future application, and what we'll refer back to for guidance on updates to existing GUIs or the creation of new GUIs. It'll be a test-case for the written English, to ensure that everything actually works together and isn't just pretty words. It should be short, yet complete and functional. It doesn't need to accomplish anything useful, it's only purpose is to excercise the guidelines. Similar to how the pyblish_qml --demo flag operates, a system stress-test.

Missing

How do we deal with multiple models? As mentioned in #436, we'll try and merge the controller (in Allzpark/Pyblish terminology) with the model, making the model both the brain and data container. But the model governs things like row and column count, and different views on different sets of data may/should have different counts. In this case, it's somewhat convenient that the second "view" - the text browser - doesn't have an official "model". But if it did, how can it share a model if it only had a single item, a single row, single column? :S

davidlatwe commented 4 years ago

But the model governs things like row and column count, and different views on different sets of data may/should have different counts. In this case, it's somewhat convenient that the second "view" - the text browser - doesn't have an official "model". But if it did, how can it share a model if it only had a single item, a single row, single column?

Happens to have an example on this one :O (should fit this context I think ?)

While I was working on the Avalon Uploader for my studio, I need two QTreeView to show different perspective (staging & uploading) on one model. And I found that it actually able to let each view has different column count. Not sure if this implementation more like a hack, at least it looks and works logical to me :P

Edit: I think the trick to share model between views was how to manage the data Role.

mottosso commented 4 years ago

I also thought of that; of making one proxy model per view. Even those that don't do anything fancy, like sorting of filtering.


     ____________________
    |                    |
    |       model        |
    |____________________|
     /        |        \
 ___/____  ___|____  ___\____
|        ||        ||        |
| proxyA || proxyB || proxyC |
|________||________||________|
    |         |         |
 ___|____  ___|____  ___|____
|        ||        ||        |
| viewA  || viewB  || viewC  |
|________||________||________|

And I think that makes sense. The proxy would be able to provide more specialised data() and setData, along with unique columns and such. They may even be able to act as more specialised business logic too? E.g.

editorModel.sortLines()
mainModel = editorModel.sourceModel()
mainModel.sortFiles()
davidlatwe commented 4 years ago

The proxy would be able to provide more specialised data() and setData, along with unique columns and such. They may even be able to act as more specialised business logic too?

Implement specialized business logic in proxies to provide processed data by that logic to view make sense to me, too.

But I am not sure about implementing the specialized setData in proxies, because isn't it just a proxy for viewing model differently ?

mottosso commented 4 years ago

But I am not sure about implementing the specialized setData

That's a good point.. doesn't feel quite right leaving this imbalance of one having data and setData, and the other only having data, but at first glance it makes sense.

BigRoy commented 4 years ago

Sorry for dropping this in here, but can we wait with merging this a day or five? I am back from vacation around Wednesday and hope to have some more time on Thursday to look into this PR and want to make sure I understand what this does and why it's there.

Being on the sidelines and reading only a glimpse of what this is doing my first hunch is to ask the question: "are we overdoing this?"

Also, the developer guide tool being in avalon.tools feels very odd to me? Can we keep that out of core or somewhere into a readme or alike? From the sidelines it feels very forced. Or do I have that feeling as I am missing some crucial part of the conversation?

To me it doesn't look like it's actually solving anything in practice but just putting in more lines of code and rules. Or will there be another PR refactoring the tools based on the ideas presented here?

Sorry of if it comes across as harsh or if I am misinterpreting things. It's definitely not intended that way, so see it as a question mostly.

mottosso commented 4 years ago

They're developer guides, like what you'd find for Blender, or Unreal, or Docker, or Go. I'd like to pass on what little knowledge I have to you, and bring out the knowledge in you in as structured of a manner as possible for all of our benefit and the benefit of the project. Avalon is growing up, and we're in a good spot to start thinking bigger than merely solving the problem right in front of us without considering the whole.

The goal is for us to both use these as reference, but also as knowledge base for when a new concept is introduced, like whether and how we should allow for and manage singletons, how we can queue tasks like in David's SFTP app, how to handle progress bars, threading and lots of concepts that run the risk of being reinvented or worse yet unconsidered for a given problem.

When we think of one idea that can be improved, we'll improve this documentation (and example). When we think of a new concept to a problem, this is where we'll document it (and implement it into the example). Then we'll have a lexicon of design patterns (and example code) to improve upon alongside the code, making Avalon scalable akin to bigger projects, like Blender, Unreal, Docker and Go.

At least, that's the dream!

davidlatwe commented 4 years ago

"Form is liberating.", so I vote "Yes" to these :)

mottosso commented 4 years ago

Thanks Roy.

I'm still not a big fan of putting the convention tool in avalon.tools.convention - it's just cluttter inside there and I'd argue it's not an actual avalon tool but it is a coding example.

Sure it's not ideal, but I didn't see an issue with it. If you've got a better idea, now's the time.

BigRoy commented 4 years ago

Sure it's not ideal, but I didn't see an issue with it. If you've got a better idea, now's the time.

I think the best location where the code can be maintained in a single location would be another repository or a specific "contribution" branch. The last seems an odd use for a branch, so I think the first is still best. The CONTRIBUTION.md file could then just link to that repository with conventions and examples.

davidlatwe commented 4 years ago

I think the best location where the code can be maintained in a single location would be another repository

Hmmm, how about put it in core/conventions/gui ? I personally lean to not separating them in different repositories. Also I think it might still be changed, so would be better if they were all in same repository when one making a PR ?

Anyway, I think the most important is easy to maintain/access for developers. :P

BigRoy commented 4 years ago

Hmmm, how about put it in core/conventions/gui ? I personally lean to not separating them in different repositories. Also I think it might still be changed, so would be better if they were all in same repository when one making a PR ?

Imagine someone working on an older version of avalon and working with older convention examples from their local deployed version. I'd rather have it be easily distinguishable that it's the latest and greatest conventions, almost like a manual/documentation page. Anyway, I've made my arguments and I'll leave the decision open for Marcus or someone else.

mottosso commented 4 years ago

I personally lean to not separating them in different repositories.

Yes, unlike Pyblish, Avalon is a monorepo; everything goes into here. Otherwise we should have a conversation about whether tools and integrations warrant their own repos too, but had I done Pyblish all over again I would have made it a monorepo.

BigRoy commented 4 years ago

Yes, unlike Pyblish, Avalon is a monorepo; everything goes into here. Otherwise we should have a conversation about whether tools and integrations warrant their own repos too, but had I done Pyblish all over again I would have made it a monorepo.

I understand, but this particular conventions repository is not required for anything to work. It's just a code example. It should be part of documentation or something. Maybe a branch actually does make sense, similar to how gh-pages is used.

mottosso commented 4 years ago

It should be part of documentation or something

Documentation is also part of this repo, in the master branch. gh-pages is implementation detail for GitHub to publish it, it isn't something you clone or write into.

Maybe docs/ is a better place for it?

So the default is stand fast, and other options are just optional ?

I put the options there to cover all cases; to not only indicate which one to use, but also indicate what else where was and why those are less ideal. I put the little checkbox in Compromise as an indicator as to which one to use but it probably wasn't apparent enough.

Happy to stick with Stand Fast, I'll update the checkbox and point it out more explicitly.

BigRoy commented 4 years ago

Maybe docs/ is a better place for it?

To me that does make more sense, sorry if I'm making quite a bold and hard opposition. Feel free to ignore if you both feel different about it.

mottosso commented 4 years ago

Maybe docs/ is a better place for it?

Thinking about this, there is docs/ which is the api ref, but then there's https://github.com/getavalon/docs which is the user docs.

Would really like to merge these into here, so docs align with the version of Avalon, and so that PRs can be made to both together. And to make it more of a monorepo. That's separate, but means docs/ might not be the best place for this after all.

What we've got is a text document with associated executable code; if I could I would have embedded the Python into the markdown, and let us somehow run the markdown directly. But that's not possible, so the only reason for putting it elsewhere is technical mumbo jumbo. Still not 100% on where.