search

getavalon / core

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

Fix #364: Make cbloader and cbsceneinventory the new defaults #388

Closed BigRoy closed 5 years ago

BigRoy commented 5 years ago

Changes:

Reference issue: #364

NOTE Currently code is untested, only refactored it for now. This way it also allows others to check their dependencies on it plus potentially discuss the naming/refactoring of "sceneinventory" as opposed to the older "manager".

mottosso commented 5 years ago

Looks good, this is the right way forwards.

BigRoy commented 5 years ago

@mottosso do you happen to know what kind of drugs the Hound sniffed? :) I'm assuming those lines are {file}.py:{line}:{column}? Is it referring to negative columns/characters? Wut?

mottosso commented 5 years ago

@mottosso do you happen to know what kind of drugs the Hound sniffed? :)

Haha, no idea. Could it be line-endings, Windows versus Linux? Didn't you have a new file-server, could that have had an effect?

BigRoy commented 5 years ago

Haha, no idea. Could it be line-endings, Windows versus Linux? Didn't you have a new file-server, could that have had an effect?

Everything is saved from a Windows workstations. Plus it's the only file showing the odd behavior and it doesn't even show any other changes than one change on line 97. Additionally, checking symbols in the file doesn't show me anything odd other than just line endings everywhere. It's really finding something magically hidden.

BigRoy commented 5 years ago

It seems it failed on the backslashes in the docstring. Seems like a bug, anyway... changed it with https://github.com/getavalon/core/pull/388/commits/e2f21d3982d33615064d8e57573dcb5951baae92 and it seems to make it pass. :)

BigRoy commented 5 years ago

@davidlatwe @tokejepsen @mkolar - tagging you guys to give this a test run - if you're referencing any of the tools in your own pipeline code note that they might need refactoring with these names changes. As such, if you are using these tools that are NOT public in Avalon API this could introduce a backwards incompatibility and will require you to update code along.

That is also described in this PR's issue #388

Also, AVALON_EARLY_ADOPTERS is effectively redundant with this change. It does not trigger any changes on behavior, but could still be used in the future if we introduce breaking changes in a point release. So note that the key is still mentioned here

This merge will likely become part of Avalon 6.0 - see: https://github.com/getavalon/core/projects/3

davidlatwe commented 5 years ago

I have tested on my end and looks working nicely after I changed those module name :) And I also made a PR in Launcher getavalon/launcher#42 for this to working.

BigRoy commented 5 years ago

And I also made a PR in Launcher getavalon/launcher#42 for this to working.

Thanks, totally forgot that action was native to the launcher currently.

mkolar commented 5 years ago

This does not affect us much. We're actually preparing (and should be done this week @iLLiCiTiT? ) a refactor of the UIs which separates models, delegates and widgets from their tools into universally usable elements for UI building in avalon. As we worked with the tools it became a bit of a mishmash of tools cross-referencing widgets from other tools, so this is an effort to make it cleaner... #upcoming

BigRoy commented 5 years ago

a refactor of the UIs which separates models, delegates and widgets from their tools into universally usable elements for UI building in avalon. As we worked with the tools it became a bit of a mishmash of tools cross-referencing widgets from other tools, so this is an effort to make it cleaner... #upcoming

Perfect! :)