Action Manager | Refactor API to future-proof property assignment on construction. #9554
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ping to reflect it to BehaviorContext. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
…ork module Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
… functions to Python via a bus. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
…itorActionHandler Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
…of nullptr handler. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
…ad of all properties explicitly. This should make the API easier to extend and more readable, albeit more verbose. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
…des to work on Test submodule of EditorPythonBindings Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>
forhalle
added
the
sig/content
cgalvan
approved these changes
May 16, 2022
lsemp3d
approved these changes
May 16, 2022
hultonha
reviewed
May 17, 2022
Comment on lines
+23
to
+33
| struct ActionContextProperties | ||
| { | ||
| AZ_RTTI(ActionProperties, "{74694A62-E3FF-43EE-98DF-D66731DC2286}"); | ||
|
|
||
| ActionContextProperties() = default; | ||
| virtual ~ActionContextProperties() = default; | ||
|
|
||
| AZStd::string m_name = ""; | ||
| }; | ||
|
|
||
| struct ActionProperties |
There was a problem hiding this comment.
These looks great! Thanks for adding 👍 (might be worth an API comment for a little more context but otherwise all good!
| EXPECT_FALSE(outcome.IsSuccess()); | ||
| } | ||
|
|
||
| TEST_F(ActionManagerFixture, RegisterActionContext) | ||
| { | ||
| auto outcome = m_actionManagerInterface->RegisterActionContext(m_widget, "o3de.context.test", "Test", ""); | ||
| auto outcome = | ||
| m_actionManagerInterface->RegisterActionContext("", "o3de.context.test", {}, m_widget); |
There was a problem hiding this comment.
nit/suggestion: I've found actually using AzToolsFramework::ActionProperties{} instead of {} makes it a lot easier to track down usages of ActionProperties - not super important but one of those things that in practice for maintenance can be useful
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Applied minor changes to API arguments ordering to uniform them between calls. Moving ActionContext and Action properties to struct to future-proof future API changes in case we need more properties to be added. Updated the tests accordingly.
No behavior changes, just changes to the API.