Skip to content

Conversation

@martint-unity
Copy link
Contributor

@martint-unity martint-unity commented Apr 7, 2021


Purpose of this PR

To create a UI and an API for adding and creating converters. These converters will help us convert projects form Built-in to URP.
This framework is mainly how to create a converter and show the data in the converter UI.
We need to have a robust and easily extendable system in which other people can make upgraders easily and use this UI/UX workflow. If another team wants to make an upgrader for GI-Bake (example) then they should easily be able to add that to this workflow.

This is how the UI looks now
image
It is not finished.

This draft PR is for the API.
Users should be able to use this and create converters.

QA:
Added the material Converter in this PR so that we have something to test. So would be good to test that on an older project. Upgrading materials. You will also need a Script define to get the tool to show. Talk to @martin-unity3d to get this setup?

The UI is not finished (will be addressed in later PR's) and if there are feedback on it then we have another document for that. again talk to @martin-unity3d to get access to that document.

Release Management Epic UR-2029

removed list to the convert method.
removed the return och list struct. Instead the converter owns those items now.
Made the bindings to the UI and the saved properties
Added category to converter class
Added which converstion stage the converter is in, like Builit-in to URP, Built-in To HDRP or your own converstion stage.
moved Converter Core to Core RP folder
Added functionality for disabling a converter
added methods for marking items for failurs or success
 that is called when switching item in the list of converter items.
made so that the icons get updated accordingly
Made a small border
added padding.
removed a visual element
Tested contextual menu for list view items
…-framework

# Conflicts:
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter.meta
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/Widget.meta
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/Widget/converter_widget.uss
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/Widget/converter_widget.uss.meta
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/Widget/converter_widget.uxml
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/Widget/converter_widget.uxml.meta
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/Widget/converter_widget_item.uxml
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/Widget/converter_widget_item.uxml.meta
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/converter_editor.uss
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/converter_editor.uss.meta
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/converter_editor.uxml
#	TestProjects/UniversalGraphicsTest_PostPro/Assets/Editor/Converter/converter_editor.uxml.meta
Copy link
Contributor

@alex-vazquez alex-vazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments

…-framework

# Conflicts:
#	com.unity.render-pipelines.core/Editor/CoreEditorStyles.cs
Added name space
change variable names on VisualElements Assets
commented choices code
@martint-unity martint-unity requested review from a team and pbbastian April 21, 2021 08:00
Copy link
Contributor

@phi-lira phi-lira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor comments.
I think we also discussed that the upgrader/converter would not expose public API until we are confident with a couple of usages to mature API?

If so, then let's make converter API internal for now until we are confident to share a nice API to users.

@erikabar erikabar self-requested a review April 26, 2021 08:33
@martint-unity martint-unity marked this pull request as ready for review April 26, 2021 09:43
@martint-unity martint-unity requested a review from a team as a code owner April 26, 2021 09:43
Copy link
Contributor

@pbbastian pbbastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, have some comments. com.unity.render-pipelines.universal/Editor/Converter/converter_widget.uss is empty and should be removed.

Removed the sace to layout method.
Fixed a bug regarding UI reloading
Copy link
Contributor

@erikabar erikabar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested if old functionality of material upgrader works with several different built-in projects, everything works smoothly, no regressions found.

@phi-lira
Copy link
Contributor

Failures are known issues, or jobs that we cancelled because they don't affect this PR.

@phi-lira phi-lira merged commit b60ac12 into master Apr 29, 2021
@phi-lira phi-lira deleted the universal/upgrader/builtin-to-urp-upgrader-framework branch April 29, 2021 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants