Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: AzLmbr Bus Event libraries for Python #57

Open
FuzzyCarter opened this issue Dec 23, 2022 · 4 comments
Open

RFC: AzLmbr Bus Event libraries for Python #57

FuzzyCarter opened this issue Dec 23, 2022 · 4 comments
Labels
rfc-feature Request for Comments for a Feature triaged/needs-information An issue which needs clarification

Comments

@FuzzyCarter
Copy link
Contributor

FuzzyCarter commented Dec 23, 2022

Summary:

An extensive library of Enums for the AzLmbr Bus Events to be used in python.

This would be a library of a raw bus event call that will pass the string for that event along instead of using string literals.

EG:

azlmbr.editor.EditorComponentAPIBus(bus.Broadcast, bus.EditorComponenetBus.AddComponentOfType, object)

vs

azlmbr.editor.EditorComponentAPIBus(bus.Broadcast, "AddComponentOfType", object)

What is the motivation for this suggestion?

Why is this important?

  • Prevents the use of using string literals that are prone to syntax errors.
  • Helps enforce SOLID and DRY software development by giving a single place for these libraries to live and usage of encapsulated data.
  • Makes updating a bus event structure easier by only having to change the value of the Enum vs finding each usage of the string in a bus event call.

What are the use cases for this suggestion?

  • Every azlmbr bus event call in the Python Automation scripts.

What should the outcome be if this suggestion is implemented?

  • An extensive library of enums to be used for calling bus events.

Suggestion design description:

  • AutoGen: Leverage the autogen to create the mappings for bus events automatically on every build
    • Advantages:
      • Mappings are updated every build allowing for up-to-date bus event mappings whenever the bus events are exposed/added/updated.
      • Eliminates automation maintenance for bus event updates due to the bus event mappings being automatically updated and remain, allowing the automation to remain stable.
      • Disadvantages:
        • Development time to plan and implement the basic autogen system for generating these.
        • This solution will require creating a pattern for changes in bus event names that will remap/point to the new event name.

Are there any alternatives to this suggestion?

  • Manual Generation: A small group of people dive into all the existing bus events and manually create an enum library of all exposed bus events.
    • Advantages:
      • Software design for this solution is minimal.
      • Updating bus events that had their name changed is easy and doesn't require a development pattern.
    • Disadvantages:
      • This will require manual maintenance when new busses are exposed, new bus events are created, or bus events change names/patterns.
      • Manual creation of enums could result in syntax errors (but are easy to fix once discovered)

What is the strategy for adoption?

  • Add the usage of libraries to the Automation Best Practices doc.
  • Add docs about the libraries.
  • Give a discord talk about the libraries.
  • Use PRs to help enforce the usage and spread of the knowledge of the libraries.
@FuzzyCarter FuzzyCarter added the rfc-feature Request for Comments for a Feature label Dec 23, 2022
@FuzzyCarter FuzzyCarter changed the title RFC: RFC: AzLmbr Bus Event Map for Python Dec 23, 2022
@FuzzyCarter FuzzyCarter changed the title RFC: AzLmbr Bus Event Map for Python RFC: AzLmbr Bus Event libraries for Python Dec 23, 2022
@lmbr-pip
Copy link

@FuzzyCarterAWS - These RFCs should be created in SIG/Testing (or other relevant SIG or TSC github) ie https://github.com/o3de/sig-testing/issues

You should work with a maintainer to migrate issue (need maintainer with permissions on SIG repository).

@jromnoa
Copy link

jromnoa commented Dec 29, 2022

Will this only be for Editor or will it be for other applications such as MaterialEditor and MaterialCanvas as well? We should make sure the buses are setup the same there as they are for Editor (I assume they are, but they may differ).

@FuzzyCarter FuzzyCarter transferred this issue from o3de/o3de Jan 3, 2023
@Kadino Kadino added the triaged/needs-information An issue which needs clarification label Jan 3, 2023
@hultonha
Copy link

hultonha commented Jan 4, 2023

I definitely like this idea in principle but I'd like more info about how this library of enums will be structured (will it map to the project structure? Will everything be added to one large file? If not will the enums map to a file per request bus? etc...).

The idea of doing this automatically is really nice and will definitely help with discoverability (and intellisense I imagine too when users type completions) but some more details on the structure/implementation would be useful to help solidify things. Thanks!

@smurly
Copy link
Contributor

smurly commented Jan 4, 2023

azlmbr.editor.EditorComponentAPIBus(bus.Broadcast, bus.EditorComponenetBus.AddComponentOfType, object)
this suggests to me that you've done import azlmbr.bus as bus and that an enum exists as azlmbr.bus.EditorComponentBus() with string values AddComponentOfType = "AddComponentOfType".
individual enums would exist for each unique bus starting with azlmbr.editor.EditorComponentAPIBus.
the enum would be defined in C++ and reflected to azlmbr python bindings. the bus itself would be modified in C++ to utilize the enum rather than static string. C++ internal use might also have to be updated for other uses
all python usage of the bus call would then be updated to reflect the enum rather than string. the enum access would look like the following (basically the same but parenthesis added at the enum call:
azlmbr.editor.EditorComponentAPIBus(azlmbr.bus.Broadcast, azlmbr.bus.EditorComponenetBus().AddComponentOfType, object)
this enum example might actually belong in azlmbr.editor rather than azlmbr.bus.

this would make for a more robust design that handles the alteration of bus method names. the downside of this is that it incurs a cost of change for every bus method string in C++ and in python in order to avoid the cost of changing bus method strings at a future time. so this only saves if we have a high rate of bus method name string change. existing search/replace tooling could be used to ease the change.
the change-over would represent a potentially breaking change to existing calls and involve a deprecation process cycle.

@FuzzyCarter FuzzyCarter removed their assignment Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc-feature Request for Comments for a Feature triaged/needs-information An issue which needs clarification
Projects
None yet
Development

No branches or pull requests

6 participants