Skip to content

StrToFloatDef can take the whole script down on a comma-decimal locale #21

Description

@furkanbsk

On a Windows machine whose regional settings use a comma as the decimal
separator (most of Europe), the Altium-side script can die mid-session. The
symptom is that the MCP server stops responding — app_ping reports the script
is not responding — and dismissing the modal Altium shows does not bring it
back, because by then the script has already unwound and the polling loop is
gone. It needs a manual StartMCPServer.

What happens

Utils.pas has two locale wrappers, FloatToJsonStr for emitting and
StrToFloatDef for parsing. Both are correct in themselves. The problem is at
the edges:

  1. Not every float is emitted through the wrapper. There are 10 call sites
    using bare FloatToStr — five in PCB.pas and five in PCBGeneric.pas.
    (Utils.pas also matches twice, but those are a comment and
    FloatToJsonStr's own body, so they are not call sites.) On a comma locale
    these produce 1,5 rather than 1.5, which is not valid JSON and is not a
    value the parse side expects.

  2. StrToFloatDef guards its conversion with Try/Except, and I believe
    that guard does not fire in DelphiScript.
    The proposed mechanism is that
    the script engine intercepts EConvertError from StrToFloat with a modal
    before the surrounding Except can run, so the default is never returned,
    the script unwinds, and the polling loop ends.

    This one is inferred, not measured, and I want to be clear about which.
    The precedent is the one your own Main.pas documents for EInOutError: an
    RTL Reset raises through the engine while a VCL EFOpenError is caught
    cleanly. EConvertError is a plausible member of the same RTL family, and
    that is an argument rather than an observation — I have not confirmed it for
    EConvertError specifically. The unit-level check in Reproduction below is
    what would settle it, and it costs nothing to run.

    Everything else in this report is measured: the call-site count, the two
    wrappers, the global mutation, and the prior art below.

So a value emitted by a bypassing site and later parsed back is enough to end
the session, and the failure looks like Altium hanging rather than like a
conversion error.

Why the two halves have to be fixed together

Both wrappers work by setting the global DecimalSeparator to '.', doing the
conversion, and restoring it. That means using the wrappers more widens the
window
in which the global is temporarily '.'. Converting the bypassing
emit sites to FloatToJsonStr is the right fix, but on its own it increases the
time spent with a mutated global; hardening the parse side on its own leaves
the malformed values being produced. Neither half is safe to ship alone.

Suggested fix, in the codebase's own idiom

StrToIntDef already solves exactly this by pre-validating: it calls
IsIntStr and returns the default without ever reaching StrToInt, so no
exception is raised and the Try/Except does not have to work. There is no
IsFloatStr counterpart.

Adding one and calling it from StrToFloatDef before the conversion makes the
float path match the integer path:

If Not IsFloatStr(S) Then
Begin
    Result := Default;
    Exit;
End;

IsFloatStr mirrors IsIntStr: optional sign, digits, at most one '.',
optional exponent — and deliberately rejects the locale comma, since the
caller forces '.' immediately afterwards.

The bypassing FloatToStr sites then want converting to FloatToJsonStr in
the same change.

Prior art in this repo

PCB.pas around line 1512 records fixing an earlier instance of this class:

"The previous OleStr->Double crash was the locale-dependent StrToFloat in the
rotation path; fixed in Utils.pas StrToFloatDef."

That fix was correct for the site it addressed. This report is that the same
class survives in the remaining bypassing emit sites — measured — and that the
parse-side guard may not contain it, for the reason set out in point 2 and with
the same caveat attached.

Reproduction

Set the Windows decimal separator to ,, run the script, and exercise any path
that round-trips a fractional value through a bare FloatToStr emit site. The
script exits and the bridge stops responding. A smaller unit-level check: call
StrToFloatDef with a comma-separated decimal inside a Try/Except in a
DelphiScript scratch script — if a modal appears rather than the default being
returned, the guard is not containing it.

I have not run that last check on a live install. It is the cheapest way to
confirm or refute the mechanism in point 2, isolated from everything else.

Environment

Altium Designer 24.9.1.31, Windows 10, Turkish regional settings
(comma decimal separator).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions