-
-
Notifications
You must be signed in to change notification settings - Fork 149
Proposed fixes for #672 #673
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
base: master
Are you sure you want to change the base?
Proposed fixes for #672 #673
Conversation
- setter of dimension Block adds new BlockRecord with unique name. - Block property writes handle.
src/ACadSharp/IO/DWG/DwgStreamWriters/DwgObjectWriter.Entities.cs
Outdated
Show resolved
Hide resolved
@@ -280,5 +283,35 @@ protected override void tableOnRemove(object sender, CollectionChangedEventArgs | |||
this._block = null; | |||
} | |||
} | |||
|
|||
|
|||
private void addToBlockRecordsTable(BlockRecord block) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's a good practice to make changes to an object that is not managed by the library itself, I would suggest to keep the old method updateTable
and add a guard that check that the block has the flag anonymous set.
If the naming of the block is an issue for any software, we can add a method that specifically changes the block name to a unique one for the current dimension, or a method in the CadDocument
that updates the name for all anonymous blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the uniqueness of the name is essential for ACadSharp.
updateTable
tries to find a BlockRecord
with the same name and returns the existing one so that the new one is to be discarded. We have to ensure that the new one is kept - with a new name.
Example:
I cloned Dimension
objects from various source DWGs to a new document. They accidenially had the same "*D..." names. Effectively I had more than one Dimension
object that were completely different referencing the same BlockRecord
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with that, I think that the solution should affect all the anonymous blocks, I'm thinking on modifying the Block record table to check for anonymous blocks and change the name to a new name when they are added into the table, we should also inform the user of this by documenting the corresponding flag inside the BlockRecord
.
Giving that this blocks are used only internally and created by the software on the spot, it would make sense that this block are partially handled by the library also, this solution offers the option for the user to change the block to a specific one (if someone wants the option) to change the dimensions for something else, at least until the graphic software takes over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referenced branch:
https://github.com/DomCR/ACadSharp/tree/anonymous-blocks-dynamic-naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is ist possible that one anonymous block is referenced by more than one object. Then it may be doubled when it is cloned. I do not remember, whether I had such a case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would not make much sense, if a dimension has its own geometry each one needs a separated block. Tables and MLeaders have to be the same.
In any case if the block is already in the table that one can be referenced again by the user.
see #672.