Repository navigation
Make assign_asset report only an assignment that holds - #35
Merged
Merged
Conversation
assign_asset printed "Assigned" without checking the result, then fell back to SetComponentProperty addressed by entity id over bus.Event, which also "succeeded". It also used azlmbr.math.Uuid() without importing azlmbr.math (dropping into a two-argument call that does not match the reflected signature), treated an unknown asset path as found because the catalog answers an invalid AssetId rather than None, and passed the id as text. It now resolves the asset with the reflected three-argument GetAssetIdByPath and checks is_valid(), finds the component (an unknown type or one not on the entity is its own error), sets the property with the AssetId and checks the outcome, and reads the property back before reporting success. Failures are asset_not_found, component_type_not_found, component_not_on_entity, set_property_failed and assign_asset_failed. Surface tests cover each failure and the read-back success; a live test assigns the engine's ground-plane mesh and refuses an unknown asset.
This file contains hidden or 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
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.
What
assign_assetcould report success for an assignment that never happened:SetComponentProperty, then fell back to aSetComponentPropertyaddressed by entity id overbus.Event. That doesn't match the reflected call, and it also "succeeded".azlmbr.math.Uuid()without importingazlmbr.math. That dropped it into a two-argumentGetAssetIdByPaththat does not match the reflected three-argument signature.AssetId, notNone.AssetId.Now
GetAssetIdByPathand checkis_valid(). An unknown path isasset_not_found, with a hint to pass the product path.component_type_not_found, and one not on the entity iscomponent_not_on_entity.AssetIdand check the outcome:set_property_failed.assign_asset_failedif it did not stick.Tests
asset_not_found.