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

Adds the ability to lock layers and sublayers at once #3646

Merged
merged 5 commits into from
Mar 8, 2024

Conversation

AramAzhari-adsk
Copy link
Collaborator

@AramAzhari-adsk AramAzhari-adsk commented Mar 4, 2024

Adds the ability to lock layers and sublayers at once

  • Updated the lockLayer command with the ability to lock sublayers.
  • Added the LayerEditorWindow query command layerHasSubLayers to track if a layer contains sublayers.
  • Added a context menu item for the layer editor to Locking a layer and its sublayers
  • Updated the layerLocking test sample to include a sublayer.
  • Updated RefreshSystemLockLayer and LockLayer to handle Undo correctly
    • RefreshSystemLockLayer now handles changing multiple layer permissions with a single Undo command.
    • Fixed Layer Lock undo when Undoing from Unlocked -> Locked
  • Added a test unit to check lock status of sublayers.
  • Updated the command documentation

Note: This PR has file change overlaps with #3643

… all affected layers.

- Also added previous lock state information to LockLayer command.
- RefreshSystemLockLayer now handles changing multiple layer permissions with a single Undo command.
- LockLayer is now capable of locking sub layers of a given layer. New parameter added to include sublayers.
- Fixed Layer Lock undo when Undoing from Unlocked -> Locked
- Updated the command documentation
- Updated the lockLayer command with the ability to lock sublayers.
- Added hasSubLayers LayerEditorWindow command.
- Added a context menu item for the layer editor to Locking a layer and its sublayers
- Updated the layerLocking test sample to include a sublayer.
- Added a test unit to check lock status of sublayers.
@@ -901,24 +930,42 @@ class RefreshSystemLockLayer : public BaseCmd
_refreshLayerSystemLock(layer);
}

// Execute lock commands
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do and undo stop at the first error and execute in the same order. For composite commands, the usual way to handle it is to record which were done so undo only undoes those that were actually successful. Also, undo usually execute in reverse order as do. Might be worthwhile to either execute as much as possible (execute all, return if any failed) or to undo on error.

For locking, these things are less critical, I don't think there will be anything bad so you can keep the simpler code that you currently have, but it is something to keep in mind.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great point!

In this implementation, the layer hierarchy is flattened and since locking only affects the local variables of each layer, the lock / unlock doesn't have to be in an order. I'll keep the error checking in mind going forward.

Copy link
Collaborator

@pierrebai-adsk pierrebai-adsk left a comment

Choose a reason for hiding this comment

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

LGTM

@AramAzhari-adsk AramAzhari-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Mar 4, 2024
@seando-adsk seando-adsk added the workflows Related to in-context workflows label Mar 8, 2024
@seando-adsk seando-adsk merged commit b735738 into dev Mar 8, 2024
13 checks passed
@seando-adsk seando-adsk deleted the azharia/EMSUSD-879/Adds-Lock-Sublayer-Command branch March 8, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge workflows Related to in-context workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants