Spun off from #1163, where the worked examples originally used Range = "[{-1,-1}, {2,2})" to define a 3x3 potential/proximity kernel and were rewritten to range(point_xy(-1s,-1s), point_xy(2s,2s)).
Problem
For point-valued units the range property takes its bounds as {a, b} string literals, in which the coordinate order is not visible in the text:
unit<spoint> k3x3: Range = "[{-1, -1}, {2, 2})"; // which of the two is the row?
The order is in fact fixed, not ambiguous: operator >> for Point<T> in rtc/dll/src/ser/PointStream.h:50 reads is >> "{" >> p.Row() >> ", " >> p.Col() >> "}", so {a, b} is {Row, Col}. Verified empirically with a deliberately non-square kernel - Range = "[{0,-1}, {1,2})" yields a 1-row x 3-col window that grows a seed cell horizontally.
But nothing in the config text says so, which is exactly the class of syntax that was banned elsewhere. The existing ban does not reach this property: dmsPoint_SetFirstCfgValue / cfg2dms_order_inplace in rtc/dll/src/geo/PointOrder.h:168-198 throwDmsErrD with "obsolete syntax for point data used. Use the point_yx operation to unambiguously define points or yx(numeric, numeric) syntax in data blocks.", but that path covers point data blocks. The range property string is read straight through the Point stream operator and never touches it, so it still parses silently and without any warning.
Proposal
Deprecate the range / cat_range property for point-valued units only, in favour of the explicit expression form:
unit<spoint> k3x3 := range(point_xy(-1s, -1s), point_xy(2s, 2s));
Confirmed to produce an identical unit (the #1163 verification configs pass unchanged after the rewrite).
Scope note: scalar ranges such as unit<uint32> u: range = "[0, 100)" have no coordinate-order concern and should keep working untouched. RangeProp in rtc/dll/src/tic/UnitClassReg.h:27-41 is a single template instantiated for every unit value type, so the deprecation needs to be conditional on the value type being a Point<T>, not applied to the property as a whole. cat_range shares the same template.
Blocker: SetDepreciated() on a property does not warn
Marking the PropDef deprecated is currently not enough. AbstrPropDef::IsDepreciated() is consulted in exactly one place - rtc/dll/src/tic/Xml/XmlTreeOut.cpp:1008 - where it merely hides the property from the detail page. No warning is emitted anywhere when a config sets a deprecated property. (The only current user of the flag is Expr vs CalcRule, rtc/dll/src/tic/TreeItemProps.cpp:210.)
Compare the operator-group machinery, which has a real two-stage path:
oper_policy::depreciated -> reportF(SeverityTypeID::ST_Warning, "depreciated operator {} used: {}."), rtc/dll/src/tic/MoreDataControllers.cpp:139
oper_policy::obsolete -> throwErrorF(...), rtc/dll/src/tic/MoreDataControllers.cpp:143
So this issue needs a property-level equivalent first. The natural choke point for .dms configs is ConfigProd::DoAnyProp (stx/dll/src/ConfigProd.cpp:665-676), which resolves the PropDef by name and calls SetValueAsCharRange - a warning there would cover every property assignment in one place. AbstrPropDef::SetValueAsCharRange itself would also catch the XML path (rtc/dll/src/tic/Xml/XmlTreeParser.cpp:144).
Suggested steps
- Add a warning path for deprecated properties (
ConfigProd::DoAnyProp and/or AbstrPropDef::SetValueAsCharRange), mirroring the operator-group warn/error split.
- Flag
range/cat_range as deprecated only for Point<T>-valued units, with a message naming the replacement: "use range(point_xy(...), point_xy(...)) to define point ranges unambiguously".
- Migrate the configs, then consider promoting the warning to an error in a later major version.
Migration impact
In the regression tree (tst): 19 occurrences across 7 files use the point-brace form, most of them potential kernels.
Operator/cfg/issue_1460.dms, Operator/cfg/OperatorDebug.dms (5x), Operator/cfg/SubItemConnect.dms, Operator/cfg/getprojectionbase.dms, Operator/cfg/issue_1372.dms, MapView/issue_219.dms
Projects/lus_demo_2023/cfg/demo/geography.dms - 9 kernel units (pot100Range ... pot02Range), the bulk of the work
A further 12 range = "..." uses in that tree are scalar and must stay as they are. The GeoDMS repo itself contains no point-brace range properties, so the change is confined to the regression configs and to user projects.
Spun off from #1163, where the worked examples originally used
Range = "[{-1,-1}, {2,2})"to define a 3x3 potential/proximity kernel and were rewritten torange(point_xy(-1s,-1s), point_xy(2s,2s)).Problem
For point-valued units the
rangeproperty takes its bounds as{a, b}string literals, in which the coordinate order is not visible in the text:The order is in fact fixed, not ambiguous:
operator >>forPoint<T>inrtc/dll/src/ser/PointStream.h:50readsis >> "{" >> p.Row() >> ", " >> p.Col() >> "}", so{a, b}is{Row, Col}. Verified empirically with a deliberately non-square kernel -Range = "[{0,-1}, {1,2})"yields a 1-row x 3-col window that grows a seed cell horizontally.But nothing in the config text says so, which is exactly the class of syntax that was banned elsewhere. The existing ban does not reach this property:
dmsPoint_SetFirstCfgValue/cfg2dms_order_inplaceinrtc/dll/src/geo/PointOrder.h:168-198throwDmsErrDwith "obsolete syntax for point data used. Use the point_yx operation to unambiguously define points or yx(numeric, numeric) syntax in data blocks.", but that path covers point data blocks. Therangeproperty string is read straight through thePointstream operator and never touches it, so it still parses silently and without any warning.Proposal
Deprecate the
range/cat_rangeproperty for point-valued units only, in favour of the explicit expression form:Confirmed to produce an identical unit (the #1163 verification configs pass unchanged after the rewrite).
Scope note: scalar ranges such as
unit<uint32> u: range = "[0, 100)"have no coordinate-order concern and should keep working untouched.RangePropinrtc/dll/src/tic/UnitClassReg.h:27-41is a single template instantiated for every unit value type, so the deprecation needs to be conditional on the value type being aPoint<T>, not applied to the property as a whole.cat_rangeshares the same template.Blocker:
SetDepreciated()on a property does not warnMarking the PropDef deprecated is currently not enough.
AbstrPropDef::IsDepreciated()is consulted in exactly one place -rtc/dll/src/tic/Xml/XmlTreeOut.cpp:1008- where it merely hides the property from the detail page. No warning is emitted anywhere when a config sets a deprecated property. (The only current user of the flag isExprvsCalcRule,rtc/dll/src/tic/TreeItemProps.cpp:210.)Compare the operator-group machinery, which has a real two-stage path:
oper_policy::depreciated->reportF(SeverityTypeID::ST_Warning, "depreciated operator {} used: {}."),rtc/dll/src/tic/MoreDataControllers.cpp:139oper_policy::obsolete->throwErrorF(...),rtc/dll/src/tic/MoreDataControllers.cpp:143So this issue needs a property-level equivalent first. The natural choke point for
.dmsconfigs isConfigProd::DoAnyProp(stx/dll/src/ConfigProd.cpp:665-676), which resolves the PropDef by name and callsSetValueAsCharRange- a warning there would cover every property assignment in one place.AbstrPropDef::SetValueAsCharRangeitself would also catch the XML path (rtc/dll/src/tic/Xml/XmlTreeParser.cpp:144).Suggested steps
ConfigProd::DoAnyPropand/orAbstrPropDef::SetValueAsCharRange), mirroring the operator-group warn/error split.range/cat_rangeas deprecated only forPoint<T>-valued units, with a message naming the replacement: "use range(point_xy(...), point_xy(...)) to define point ranges unambiguously".Migration impact
In the regression tree (
tst): 19 occurrences across 7 files use the point-brace form, most of them potential kernels.Operator/cfg/issue_1460.dms,Operator/cfg/OperatorDebug.dms(5x),Operator/cfg/SubItemConnect.dms,Operator/cfg/getprojectionbase.dms,Operator/cfg/issue_1372.dms,MapView/issue_219.dmsProjects/lus_demo_2023/cfg/demo/geography.dms- 9 kernel units (pot100Range...pot02Range), the bulk of the workA further 12
range = "..."uses in that tree are scalar and must stay as they are. The GeoDMS repo itself contains no point-bracerangeproperties, so the change is confined to the regression configs and to user projects.