forked from 1b-agent/opencadstudio-upstream
feat(python): block contents (create_entity(..., block=))
BlockEntityAdd adds a validated entity to a block definition (owner set by the host, same validation as a model-space entity, self-nesting inserts refused), reusing the scene's block-editor routing. Deleting a block member now also drops its handle from the block record. Real-IPC audit with an attribute definition inside a block, DWG/DXF persistence and undo; IPC and Python unit tests; docs. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
1 parent
8d20d280fe
commit
b1edc9f578
8 files changed
+251
-7
No files matched your search
@@ -142,6 +142,11 @@ pub enum TableOperation {
|
||||
BlockRename { from: String, to: String },
|
||||
/// Delete an unreferenced block definition and its contents.
|
||||
BlockDelete { name: String },
|
||||
/// Add a new entity to a block definition, validated exactly as a new
|
||||
/// model-space entity is. The entity's owner is set by the host. Returns the
|
||||
/// new entity's handle. Edit and delete members with the ordinary entity
|
||||
/// operations.
|
||||
BlockEntityAdd { block: String, entity: EntityType },
|
||||
}
|
||||
|
||||
/// The style tables `TableOperation` can rename, delete or make current.
|
||||
|
||||
@@ -141,6 +141,10 @@ mod tests {
|
||||
TableOperation::BlockModify { name: "B".into(), description: None, explodable: Some(false), scale_uniformly: Some(true) },
|
||||
TableOperation::BlockRename { from: "B".into(), to: "C".into() },
|
||||
TableOperation::BlockDelete { name: "C".into() },
|
||||
TableOperation::BlockEntityAdd {
|
||||
block: "C".into(),
|
||||
entity: acadrust::EntityType::Line(acadrust::entities::Line::new()),
|
||||
},
|
||||
] {
|
||||
let bytes = bincode::serialize(&PluginRequest::TableOperation { operation: operation.clone() }).unwrap();
|
||||
assert!(matches!(bincode::deserialize::<PluginRequest>(&bytes).unwrap(),
|
||||
|
||||
@@ -227,3 +227,16 @@ unit tests. One more cadcodec finding (DXF drops the block description) is pinne
|
||||
editing a definition's contents in place, and attribute definitions inside a scripted block. Next: linetypes and
|
||||
layouts, then headless commands.
|
||||
|
||||
### 2026-09-21: block contents (`create_entity(..., block=)`)
|
||||
|
||||
`TableOperation::BlockEntityAdd` adds a validated entity to a definition: the host sets the owner, runs the same
|
||||
validation and reference binding as a new model-space entity, refuses viewports, raster images, block markers,
|
||||
nested attributes and self-nesting inserts (direct or transitive), then reuses the scene's block-editor routing so
|
||||
the entity gets normal preparation. Editing and deleting members already worked through the entity API; deleting a
|
||||
member now also drops its handle from the block record, because the core document leaves a removed entity
|
||||
listed (`entity_handles`). An entity-delta undo of an add still leaves the handle listed (documented scene behaviour),
|
||||
so consumers must treat `entity_handles` as candidates and confirm with `get_entity`; the Python block records
|
||||
already do. Evidence: `audit_python_block_contents_over_real_ipc` (9 refusals; create, edit, delete in a block;
|
||||
attribute definition inside a block; nested insert; live/DWG/DXF persistence; undo), IPC and Python unit tests.
|
||||
Next: linetypes and layouts, then headless commands.
|
||||
|
||||
@@ -209,9 +209,20 @@ block, a viewport, a non-finite base point, renaming or deleting layout,
|
||||
anonymous or externally referenced blocks, and a case-only rename. Deleting also
|
||||
removes the definition's contents. Records list user blocks only (`handle`,
|
||||
`name`, `description`, `explodable`, `scale_uniformly`, `base_point`,
|
||||
`insert_count`, `entities` as `{handle, kind}`). Editing a definition's contents
|
||||
in place and creating attribute definitions inside a block from a script are not
|
||||
available yet. A DXF save drops the block description (cadcodec); DWG keeps it.
|
||||
`insert_count`, `entities` as `{handle, kind}`). A DXF save drops the block
|
||||
description (cadcodec); DWG keeps it.
|
||||
|
||||
**Block contents.** Add to a definition with `create_entity(kind, block="Part",
|
||||
...)`. It is validated exactly like a model-space entity and the host sets the
|
||||
owner (so attribute definitions work: `create_entity("AttributeDefinition",
|
||||
block="Part", tag=..., prompt=..., default_value=..., insertion_point=...,
|
||||
height=...)`). Members are read, edited (`entity.end = (8, 0, 0)` inside
|
||||
`doc.transaction(...)`) and deleted (`doc.delete_entity`) with the ordinary entity
|
||||
API, using the handles in `blocks["Part"]["entities"]`. Refused: an unknown,
|
||||
layout, anonymous or external block, an invalid entity, a viewport, raster image,
|
||||
block marker or nested attribute, an entity on a locked layer, and an insert that
|
||||
would nest a block inside itself directly or through other blocks. Adding while the
|
||||
block editor is open is refused. Each add or delete is one undo step.
|
||||
|
||||
### Historical command-replay experiment (not in the default build)
|
||||
|
||||
|
||||
@@ -473,11 +473,17 @@ class _Document:
|
||||
def transaction(self, label):
|
||||
return _Transaction(self, label)
|
||||
|
||||
def create_entity(self, kind, **properties):
|
||||
"""Create one mapped entity and return its live document descriptor."""
|
||||
def create_entity(self, kind, block=None, **properties):
|
||||
"""Create one mapped entity and return its live document descriptor.
|
||||
With `block="Name"` the entity is added to that block definition
|
||||
instead of the drawing (its owner is set by the host)."""
|
||||
if "kind" in properties or "handle" in properties:
|
||||
raise ValueError("kind and handle are managed by create_entity")
|
||||
handle = ocs.add(dict(kind=kind, **_coerce_points(kind, properties)))
|
||||
entity = dict(kind=kind, **_coerce_points(kind, properties))
|
||||
if block is not None:
|
||||
handle = ocs.add_to_block(str(block), entity)
|
||||
else:
|
||||
handle = ocs.add(entity)
|
||||
return self.entities[handle]
|
||||
|
||||
def delete_entity(self, entity):
|
||||
|
||||
@@ -4263,6 +4263,31 @@ mod ocs {
|
||||
.ok_or_else(|| vm.new_runtime_error("ocs: not running inside a PY_ command".to_owned()))?
|
||||
}
|
||||
|
||||
/// Add a new entity, built from `entity` like `add`, to the block definition
|
||||
/// `block`. Returns the new entity's handle.
|
||||
#[cfg(feature = "experimental-host-model")]
|
||||
#[pyfunction]
|
||||
fn add_to_block(block: String, entity: PyObjectRef, vm: &VirtualMachine) -> PyResult<u64> {
|
||||
let dict = entity.try_into_value::<rustpython_vm::builtins::PyDictRef>(vm)?;
|
||||
let mut built = dict_to_entity(&dict, vm)?;
|
||||
let result = host_ctx::with_host(|host| -> PyResult<Result<Handle, String>> {
|
||||
let owner = host
|
||||
.document()
|
||||
.block_records
|
||||
.get(block.trim())
|
||||
.map(|record| record.handle)
|
||||
.ok_or_else(|| vm.new_runtime_error(format!("ocs.add_to_block: block {block:?} does not exist")))?;
|
||||
built.common_mut().owner_handle = owner;
|
||||
ocs_plugin_api::entity_coverage::validate_new_canvas_entity(&built)
|
||||
.map_err(|error| vm.new_value_error(error))?;
|
||||
Ok(host.table_operation(ocs_plugin_api::host::TableOperation::BlockEntityAdd { block, entity: built }))
|
||||
})
|
||||
.ok_or_else(|| vm.new_runtime_error("ocs: not running inside a PY_ command".to_owned()))??;
|
||||
result
|
||||
.map(|handle| handle.value())
|
||||
.map_err(|error| vm.new_runtime_error(format!("ocs.add_to_block: {error}")))
|
||||
}
|
||||
|
||||
/// Update `handle` by merging `entity`'s keys onto its *current* value —
|
||||
/// a key `entity` doesn't mention is left exactly as it was, not reset to
|
||||
/// a type default (Felix's Phase 1 review: reconstructing from a partial
|
||||
|
||||
@@ -41,6 +41,7 @@ class DocumentModelTests(unittest.TestCase):
|
||||
style_operation=lambda kind, op, name, options: self.calls.append((kind, op, name, options)) or 9,
|
||||
block_records=lambda: [{"name": n, "entities": []} for n in ("Widget", "Gadget")],
|
||||
block_operation=lambda op, name, options: self.calls.append(("block", op, name, options)) or 9,
|
||||
add_to_block=lambda block, entity: self.calls.append(("add_to_block", block, entity)) or 1,
|
||||
)
|
||||
namespace = {"ocs": self.ocs}
|
||||
exec(MODEL.read_text(), namespace)
|
||||
@@ -96,6 +97,14 @@ class DocumentModelTests(unittest.TestCase):
|
||||
dim.set_current("Standard")
|
||||
self.assertEqual(self.calls[-1], ("dim", "set_current", "Standard", None))
|
||||
|
||||
def test_create_entity_in_block(self):
|
||||
self.doc.create_entity("Line", block="Widget", start={"x": 0, "y": 0, "z": 0}, end={"x": 1, "y": 0, "z": 0})
|
||||
self.assertEqual(self.calls[-1][:2], ("add_to_block", "Widget"))
|
||||
self.assertEqual(self.calls[-1][2]["kind"], "Line")
|
||||
self.assertNotIn("block", self.calls[-1][2])
|
||||
self.doc.create_entity("Line", start={"x": 0, "y": 0, "z": 0}, end={"x": 1, "y": 0, "z": 0})
|
||||
self.assertEqual(self.calls[-1][0], "add")
|
||||
|
||||
def test_blocks(self):
|
||||
blocks = self.doc.blocks
|
||||
self.assertIn("WIDGET", blocks)
|
||||
|
||||
+172
-1
@@ -503,9 +503,18 @@ impl<'a> HostSession<'a> {
|
||||
}
|
||||
return removed;
|
||||
}
|
||||
let owner = self.document().get_entity(handle).map(|entity| entity.common().owner_handle);
|
||||
self.app.tabs[self.tab].scene.erase_entities(&[handle]);
|
||||
let removed = self.document().get_entity(handle).is_none();
|
||||
if removed {
|
||||
// The document keeps a block record's member list separately from the
|
||||
// entity store, and removing an entity leaves its handle listed; drop
|
||||
// it so a definition never lists a member that no longer exists.
|
||||
if let Some(owner) = owner {
|
||||
if let Some(record) = self.document_mut().block_records.iter_mut().find(|record| record.handle == owner) {
|
||||
record.entity_handles.retain(|member| *member != handle);
|
||||
}
|
||||
}
|
||||
self.publish_document_view();
|
||||
}
|
||||
removed
|
||||
@@ -1248,7 +1257,8 @@ impl<'a> HostSession<'a> {
|
||||
other @ (TableOperation::BlockCreate { .. }
|
||||
| TableOperation::BlockModify { .. }
|
||||
| TableOperation::BlockRename { .. }
|
||||
| TableOperation::BlockDelete { .. }) => self.block_operation(other),
|
||||
| TableOperation::BlockDelete { .. }
|
||||
| TableOperation::BlockEntityAdd { .. }) => self.block_operation(other),
|
||||
other => self.style_operation(other),
|
||||
}
|
||||
}
|
||||
@@ -1336,6 +1346,42 @@ impl<'a> HostSession<'a> {
|
||||
self.finish_block_change();
|
||||
Ok(handle)
|
||||
}
|
||||
TableOperation::BlockEntityAdd { block, mut entity } => {
|
||||
let (name, record_handle) = editable_block(self.document(), block.trim())?;
|
||||
if matches!(
|
||||
entity,
|
||||
E::Viewport(_) | E::Block(_) | E::BlockEnd(_) | E::AttributeEntity(_) | E::Unknown(_) | E::RasterImage(_)
|
||||
) {
|
||||
return Err("an entity of this kind cannot be added to a block from a script".to_owned());
|
||||
}
|
||||
if self.app.tabs[self.tab].scene.block_edit_block.is_some() {
|
||||
return Err("finish the open block editor before adding entities to a block".to_owned());
|
||||
}
|
||||
if let E::Insert(insert) = &entity {
|
||||
if insert.block_name.eq_ignore_ascii_case(&name) || block_nests(self.document(), &insert.block_name, &name, 0) {
|
||||
return Err(format!("inserting {:?} into {name:?} would nest a block inside itself", insert.block_name));
|
||||
}
|
||||
}
|
||||
entity.common_mut().owner_handle = record_handle;
|
||||
ocs_plugin_api::entity_coverage::validate_new_canvas_entity(&entity)?;
|
||||
ocs_plugin_api::entity_coverage::bind_canvas_entity_references(self.document(), &mut entity)?;
|
||||
if self.document().layers.get(&entity.common().layer).is_some_and(|layer| layer.is_locked()) {
|
||||
return Err(format!("layer {:?} is locked", entity.common().layer));
|
||||
}
|
||||
self.push_undo("Add to block");
|
||||
entity.common_mut().handle = Handle::NULL;
|
||||
entity.common_mut().owner_handle = record_handle;
|
||||
// The scene routes an add into the block being edited; reuse that
|
||||
// path so the entity gets the same preparation as any new entity.
|
||||
self.app.tabs[self.tab].scene.block_edit_block = Some(record_handle);
|
||||
let handle = self.add_entity(entity);
|
||||
self.app.tabs[self.tab].scene.block_edit_block = None;
|
||||
if handle.is_null() {
|
||||
return Err(format!("the entity could not be added to block {name:?}"));
|
||||
}
|
||||
self.finish_block_change();
|
||||
Ok(handle)
|
||||
}
|
||||
TableOperation::BlockModify { name, description, explodable, scale_uniformly } => {
|
||||
let (name, handle) = editable_block(self.document(), name.trim())?;
|
||||
if description.is_none() && explodable.is_none() && scale_uniformly.is_none() {
|
||||
@@ -1666,6 +1712,22 @@ impl<'a> HostSession<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
/// True when block `outer` (transitively) contains an insert of `target`.
|
||||
fn block_nests(doc: &CadDocument, outer: &str, target: &str, depth: usize) -> bool {
|
||||
if depth > 64 {
|
||||
return true;
|
||||
}
|
||||
let Some(record) = doc.block_records.get(outer) else {
|
||||
return false;
|
||||
};
|
||||
record.entity_handles.iter().filter_map(|h| doc.get_entity(*h)).any(|entity| match entity {
|
||||
EntityType::Insert(insert) => {
|
||||
insert.block_name.eq_ignore_ascii_case(target) || block_nests(doc, &insert.block_name, target, depth + 1)
|
||||
}
|
||||
_ => false,
|
||||
})
|
||||
}
|
||||
|
||||
/// A layer name AutoCAD would accept: 1-255 characters, none of `<>/\":;?*|=\``.
|
||||
fn validated_layer_name(raw: &str) -> Result<String, String> {
|
||||
validated_symbol_name(raw, "a layer name")
|
||||
@@ -7139,6 +7201,115 @@ B.delete('Widget2')
|
||||
assert_eq!(members("Widget2"), Some(1), "undo restored Widget2");
|
||||
}
|
||||
|
||||
/// Block contents through Python over the real runner: add entities
|
||||
/// (including an attribute definition) to a definition, edit and delete them
|
||||
/// with the ordinary entity API, refuse bad requests, persist through
|
||||
/// DWG/DXF and undo.
|
||||
#[test]
|
||||
fn audit_python_block_contents_over_real_ipc() {
|
||||
let Some(plugin_path) = std::env::var_os("OCS_TEST_PYTHON_PLUGIN") else {
|
||||
return;
|
||||
};
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
app.tabs[0].is_start = false;
|
||||
let mut host = HostSession::new(&mut app, 0);
|
||||
let process = ocs_plugin_api::process::PluginProcess::spawn(
|
||||
std::path::Path::new(&plugin_path), &mut host, crate::plugin::v4_support::notification_handler(),
|
||||
).unwrap();
|
||||
let dir = std::env::temp_dir();
|
||||
let run = |host: &mut HostSession<'_>, tag: &str, body: &str| {
|
||||
let script = dir.join(format!("ocs_bcontents_{tag}_{}.py", std::process::id()));
|
||||
std::fs::write(&script, format!(concat!(
|
||||
"def P(x, y, z): return {{'x': x, 'y': y, 'z': z}}\n",
|
||||
"doc = ocs.active_document\nB = doc.blocks\nL = doc.layers\n",
|
||||
"refused, accepted = [], []\n",
|
||||
"def check(tag, fn):\n",
|
||||
" try:\n fn()\n except (RuntimeError, TypeError, ValueError):\n refused.append(tag)\n",
|
||||
" else:\n accepted.append(tag)\n",
|
||||
"try:\n{body}\n",
|
||||
"except Exception as error:\n",
|
||||
" accepted.append('SCRIPTERROR_' + ''.join(c if c.isalnum() else '_' for c in str(error))[:120])\n",
|
||||
"L.create('REPORT ' + str(len(refused)) + ' ~ ' + ' '.join(accepted))\n",
|
||||
), body = body.lines().map(|line| format!(" {line}")).collect::<Vec<_>>().join("\n"))).unwrap();
|
||||
assert!(process.dispatch(host, &format!("PY_RUN {}", script.display()), &mut |_| {}).unwrap());
|
||||
let _ = std::fs::remove_file(&script);
|
||||
};
|
||||
let report = |document: &CadDocument| -> (usize, Vec<String>) {
|
||||
let name = document.layers.iter().map(|l| l.name.clone()).filter(|n| n.starts_with("REPORT")).last()
|
||||
.expect("script report layer");
|
||||
let (refused, accepted) = name["REPORT".len()..].split_once('~').unwrap();
|
||||
(refused.trim().parse().unwrap(), accepted.split_whitespace().map(str::to_owned).collect())
|
||||
};
|
||||
|
||||
run(&mut host, "build", r#"
|
||||
seed = doc.create_entity('Circle', center=P(0, 0, 0), radius=1)
|
||||
B.create('Part', [seed], erase_originals=True)
|
||||
line = doc.create_entity('Line', start=P(0, 0, 0), end=P(5, 0, 0), block='Part')
|
||||
arc = doc.create_entity('Arc', center=P(0, 0, 0), radius=5.0, start_angle=0.0, end_angle=1.5, block='Part')
|
||||
tag = doc.create_entity('AttributeDefinition', block='Part', tag='PART_NO', prompt='Part number', default_value='PN-001', insertion_point=P(1, 2, 0), height=2.5)
|
||||
with doc.transaction('Lengthen'):
|
||||
line.end = (8, 0, 0)
|
||||
doc.delete_entity(arc)
|
||||
B.create('Outer', [doc.create_entity('Circle', center=P(0, 0, 0), radius=1)], erase_originals=True)
|
||||
doc.create_entity('Insert', block_name='Part', insert_point=P(0, 0, 0), block='Outer')
|
||||
check('missing_block', lambda: doc.create_entity('Line', start=P(0, 0, 0), end=P(1, 0, 0), block='Nope'))
|
||||
check('layout_block', lambda: doc.create_entity('Line', start=P(0, 0, 0), end=P(1, 0, 0), block='*Model_Space'))
|
||||
check('zero_circle', lambda: doc.create_entity('Circle', center=P(1, 1, 0), radius=0, block='Part'))
|
||||
check('viewport', lambda: doc.create_entity('Viewport', block='Part'))
|
||||
check('bad_tag', lambda: doc.create_entity('AttributeDefinition', block='Part', tag=' ', prompt='p', default_value='', insertion_point=P(0, 0, 0), height=1))
|
||||
check('bad_height', lambda: doc.create_entity('AttributeDefinition', block='Part', tag='T', prompt='p', default_value='', insertion_point=P(0, 0, 0), height=0))
|
||||
check('self_insert', lambda: doc.create_entity('Insert', block_name='Part', insert_point=P(0, 0, 0), block='Part'))
|
||||
check('cycle', lambda: doc.create_entity('Insert', block_name='Outer', insert_point=P(0, 0, 0), block='Part'))
|
||||
check('unknown_insert_block', lambda: doc.create_entity('Insert', block_name='Ghost', insert_point=P(0, 0, 0), block='Part'))
|
||||
"#);
|
||||
let (refused, accepted) = report(host.document());
|
||||
assert!(accepted.is_empty(), "accepted invalid requests: {accepted:?}");
|
||||
assert_eq!(refused, 9);
|
||||
|
||||
let check_state = |label: &str, document: &CadDocument| {
|
||||
let record = document.block_records.get("Part").unwrap_or_else(|| panic!("{label}: Part missing"));
|
||||
let members: Vec<EntityType> = document.entities()
|
||||
.filter(|e| e.common().owner_handle == record.handle && !matches!(e, EntityType::Block(_) | EntityType::BlockEnd(_)))
|
||||
.cloned().collect();
|
||||
assert_eq!(members.len(), 3, "{label}: circle, line, attribute definition (the arc was deleted): {:?}",
|
||||
members.iter().map(|e| format!("{e:?}").chars().take(10).collect::<String>()).collect::<Vec<_>>());
|
||||
let line = members.iter().find_map(|e| match e { EntityType::Line(l) => Some(l.clone()), _ => None }).unwrap_or_else(|| panic!("{label}: line"));
|
||||
assert_eq!((line.end.x, line.end.y), (8.0, 0.0), "{label}: edited inside the block");
|
||||
assert!(members.iter().any(|e| matches!(e, EntityType::Circle(_))), "{label}: circle from the seed");
|
||||
let attdef = members.iter().find_map(|e| match e { EntityType::AttributeDefinition(a) => Some(a.clone()), _ => None }).unwrap_or_else(|| panic!("{label}: attdef"));
|
||||
assert_eq!((attdef.tag.as_str(), attdef.prompt.as_str(), attdef.default_value.as_str()), ("PART_NO", "Part number", "PN-001"), "{label}");
|
||||
assert!(!members.iter().any(|e| matches!(e, EntityType::Arc(_))), "{label}: arc deleted");
|
||||
let outer = document.block_records.get("Outer").unwrap_or_else(|| panic!("{label}: Outer missing"));
|
||||
let nested = document.entities().any(|e| matches!(e, EntityType::Insert(i) if i.block_name == "Part" && i.common.owner_handle == outer.handle));
|
||||
assert!(nested, "{label}: Part is inserted inside Outer");
|
||||
};
|
||||
check_state("live", host.document());
|
||||
let dwg = crate::io::load_bytes("bc.dwg", acadrust::DwgWriter::write_to_vec(host.document()).unwrap()).unwrap();
|
||||
let dxf = crate::io::load_bytes("bc.dxf", acadrust::DxfWriter::new(host.document()).write_to_vec().unwrap()).unwrap();
|
||||
check_state("DWG", &dwg);
|
||||
check_state("DXF", &dxf);
|
||||
|
||||
// U: one more add, then the report layer; undoing both leaves the block as before.
|
||||
run(&mut host, "extra", "doc.create_entity('Circle', center=P(9, 9, 0), radius=2, block='Part')");
|
||||
assert_eq!(report(host.document()).1.len(), 0);
|
||||
{
|
||||
let document = host.document();
|
||||
let record = document.block_records.get("Part").unwrap();
|
||||
let live: Vec<_> = record.entity_handles.iter().map(|h| (h.value(), document.get_entity(*h).is_some())).collect();
|
||||
assert_eq!(record.entity_handles.len(), 4, "record entity_handles (handle, exists): {live:?}");
|
||||
}
|
||||
drop(process);
|
||||
drop(host);
|
||||
app.finish_pending_history(0);
|
||||
app.undo_steps(2);
|
||||
// Membership bookkeeping is intrinsic to an add and stays listed across an entity-delta
|
||||
// undo (see `Scene::add_entity_internal`), so count the entities that really exist.
|
||||
let document = &app.tabs[0].scene.document;
|
||||
let part = document.block_records.get("Part").unwrap();
|
||||
let existing = part.entity_handles.iter().filter(|h| document.get_entity(**h).is_some()).count();
|
||||
assert_eq!(existing, 3, "undo removed the added member");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn audit_python_raster_image_definition_linkage_over_real_ipc() {
|
||||
use acadrust::objects::ObjectType;
|
||||
|
||||
Reference in new issue
Block a user