fix(vt): round 3 item 6 — SetOptionInAll parity with bk.a + retail message/failure text
bk.a (refs/vtank/decompiled/bk.cs:6-35) appends a brand-new Settings row
([0]=name, [1]=value, every other column a void gy() cell) when a .usd
file's Settings table has no row for that name at all, and reports
"Done saving setting X to all profiles. (Changed N profiles)" with N =
every .usd file scanned. MossTankProfileStore.SetOptionInAll instead
silently skipped any file missing the row, and MossTankPanel's message
("Set option X in N profile(s) = value") matched neither retail text nor
count semantics.
Fixed SetOptionInAll to append (VtankCell { Tag = "0" } for every column,
then overwrite [nameColumn]/[valueColumn]) and to return the total scanned
file count instead of a per-row "changed" tally; changed the setinall
success message to retail's exact text.
Also added real type validation to /vt opt set|setinall: TryParseOptionValue
now takes the catalog's declared VtankSettingValueType and fails with
retail's exact "Option set: Invalid value specified. Proper type of X is
Y." text (refs/vtank/decompiled/uTank2/PluginCore.cs:5501,5508,5612) when
the typed value doesn't parse as that type — previously any non-empty
string silently succeeded regardless of the option's real type. The
Advanced Options editor keeps the original lax free-form parse (no
catalog type to check against there).
Mutation: reverted MossTankProfileStore.cs/MossTankCommands.cs to HEAD
(keeping only the new tests) and ran all four new/changed tests — all
four failed (old "Set option..." message text, no append, no type
validation) — confirming they exercise the bug before the fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
7aee57fa22
commit
cc5b71a478
3 changed files with 230 additions and 24 deletions
|
|
@ -425,17 +425,28 @@ internal sealed partial class MossTankPanel
|
|||
WriteVtank("Option set: Invalid option specified.");
|
||||
return;
|
||||
}
|
||||
if (rawValue.Length == 0 || !TryParseOptionValue(rawValue, out ExpressionValue value))
|
||||
canonical = VtankOptionCatalog.Canonical(name);
|
||||
VtankSettingValueType declaredType = VtankOptionCatalog.DeclaredType(canonical);
|
||||
if (rawValue.Length == 0 || !TryParseOptionValue(rawValue, declaredType, out ExpressionValue value))
|
||||
{
|
||||
WriteVtank("Option set: Invalid value specified.");
|
||||
// Round 3 item 6: retail's exact failure text
|
||||
// (refs/vtank/decompiled/uTank2/PluginCore.cs:5501,5508,5612)
|
||||
// names the option and the CLR type its Settings row
|
||||
// actually declares (gy's own m_a, ToString()'d).
|
||||
WriteVtank(
|
||||
"Option set: Invalid value specified. Proper type of "
|
||||
+ canonical + " is " + DeclaredClrTypeName(declaredType) + ".");
|
||||
return;
|
||||
}
|
||||
canonical = VtankOptionCatalog.Canonical(name);
|
||||
SetMetaOption(canonical, value);
|
||||
if (operation.Equals("setinall", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
// Round 3 item 6: retail's exact bk.a text
|
||||
// (refs/vtank/decompiled/bk.cs:32) — no "= value" suffix,
|
||||
// and the count is every .usd file scanned, not just the
|
||||
// ones that already had a row for this name.
|
||||
int count = _profiles.SetOptionInAll(canonical, _allSettings);
|
||||
WriteVtank($"Set option {canonical} in {count} profile(s) = {GetMetaOption(canonical).ToDisplayString()}");
|
||||
WriteVtank($"Done saving setting {canonical} to all profiles. (Changed {count} profiles)");
|
||||
}
|
||||
else
|
||||
{
|
||||
|
|
@ -965,22 +976,107 @@ internal sealed partial class MossTankPanel
|
|||
return result;
|
||||
}
|
||||
|
||||
private static bool TryParseOptionValue(string source, out ExpressionValue value)
|
||||
/// <summary>
|
||||
/// Free-form parse (no catalog type to check against) for callers like
|
||||
/// the Advanced Options editor, which can target names outside the 137-
|
||||
/// row catalog: best-effort bool, then number, then string.
|
||||
/// </summary>
|
||||
private static bool TryParseOptionValue(string source, out ExpressionValue value) =>
|
||||
TryParseOptionValue(source, declaredType: null, out value);
|
||||
|
||||
/// <summary>
|
||||
/// Round 3 item 6: <c>/vt opt set</c>/<c>setinall</c> validate the typed
|
||||
/// value against the catalog's OWN declared <see cref="VtankSettingValueType"/>
|
||||
/// (retail's <c>eSettingValueType</c>) rather than accepting anything —
|
||||
/// a value that does not parse as that type fails with retail's exact
|
||||
/// "Proper type of X is Y." text (<see cref="HandleOptionCommand"/>).
|
||||
/// <paramref name="declaredType"/> is <see langword="null"/> only for
|
||||
/// the free-form Advanced Options editor, which keeps the original lax
|
||||
/// best-effort behavior.
|
||||
/// </summary>
|
||||
private static bool TryParseOptionValue(
|
||||
string source,
|
||||
VtankSettingValueType? declaredType,
|
||||
out ExpressionValue value)
|
||||
{
|
||||
if (bool.TryParse(source, out bool boolean))
|
||||
if (source.Length == 0)
|
||||
{
|
||||
value = ExpressionValue.Boolean(boolean);
|
||||
return true;
|
||||
value = default;
|
||||
return false;
|
||||
}
|
||||
if (double.TryParse(source, NumberStyles.Float, CultureInfo.InvariantCulture, out double number))
|
||||
switch (declaredType)
|
||||
{
|
||||
value = ExpressionValue.Number(number);
|
||||
return true;
|
||||
case VtankSettingValueType.Bool:
|
||||
if (bool.TryParse(source, out bool boolean))
|
||||
{
|
||||
value = ExpressionValue.Boolean(boolean);
|
||||
return true;
|
||||
}
|
||||
value = default;
|
||||
return false;
|
||||
case VtankSettingValueType.Int:
|
||||
case VtankSettingValueType.Enum:
|
||||
if (int.TryParse(
|
||||
source, NumberStyles.Integer, CultureInfo.InvariantCulture, out int integer))
|
||||
{
|
||||
value = ExpressionValue.Number(integer);
|
||||
return true;
|
||||
}
|
||||
value = default;
|
||||
return false;
|
||||
case VtankSettingValueType.Double:
|
||||
case VtankSettingValueType.Single:
|
||||
if (double.TryParse(
|
||||
source, NumberStyles.Float, CultureInfo.InvariantCulture, out double number))
|
||||
{
|
||||
value = ExpressionValue.Number(number);
|
||||
return true;
|
||||
}
|
||||
value = default;
|
||||
return false;
|
||||
case VtankSettingValueType.String:
|
||||
value = ExpressionValue.String(source);
|
||||
return true;
|
||||
default:
|
||||
// No declared type (free-form editor) or VtankSettingValueType.Custom
|
||||
// (the one non-scalar row, RechargeHandlerSet, never reached
|
||||
// through /vt opt set): keep the original lax parse.
|
||||
if (bool.TryParse(source, out bool freeBoolean))
|
||||
{
|
||||
value = ExpressionValue.Boolean(freeBoolean);
|
||||
return true;
|
||||
}
|
||||
if (double.TryParse(
|
||||
source, NumberStyles.Float, CultureInfo.InvariantCulture, out double freeNumber))
|
||||
{
|
||||
value = ExpressionValue.Number(freeNumber);
|
||||
return true;
|
||||
}
|
||||
value = ExpressionValue.String(source);
|
||||
return true;
|
||||
}
|
||||
value = ExpressionValue.String(source);
|
||||
return source.Length != 0;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The CLR type name retail's exact failure text names
|
||||
/// (<c>refs/vtank/decompiled/uTank2/PluginCore.cs:5501,5508,5612</c>:
|
||||
/// <c>cw2[1].b().ToString()</c> — the Settings row's underlying
|
||||
/// <c>gy</c> cell type, <see cref="Type.ToString"/>'d). Retail stores an
|
||||
/// Enum-declared row as a plain int cell (confirmed against the
|
||||
/// UseArcs row in the shipped defaultsettings.usd fixture), so it also
|
||||
/// reads "System.Int32".
|
||||
/// </summary>
|
||||
private static string DeclaredClrTypeName(VtankSettingValueType type) => type switch
|
||||
{
|
||||
VtankSettingValueType.Bool => "System.Boolean",
|
||||
VtankSettingValueType.Double => "System.Double",
|
||||
VtankSettingValueType.Int => "System.Int32",
|
||||
VtankSettingValueType.Single => "System.Single",
|
||||
VtankSettingValueType.String => "System.String",
|
||||
VtankSettingValueType.Enum => "System.Int32",
|
||||
_ => "System.Object",
|
||||
};
|
||||
|
||||
private bool TryParseCoordinates(string source, out PluginNavigationPosition position)
|
||||
{
|
||||
position = default;
|
||||
|
|
|
|||
|
|
@ -328,13 +328,22 @@ internal sealed class MossTankProfileStore
|
|||
}
|
||||
|
||||
/// <summary>
|
||||
/// VTank's <c>opt setinall</c>: patch every known <c>.usd</c> file's
|
||||
/// Settings row for <paramref name="name"/> (including the currently
|
||||
/// bound character's own file even if it has never been saved before),
|
||||
/// leaving every other row and every other table untouched. The live
|
||||
/// value has already been applied by the caller (<c>SetMetaOption</c>)
|
||||
/// before this runs; this only persists that same value into every
|
||||
/// profile file.
|
||||
/// VTank's <c>opt setinall</c> (<c>bk.a</c>, <c>refs/vtank/decompiled/bk.cs:6-35</c>):
|
||||
/// patch every known <c>.usd</c> file's Settings row for
|
||||
/// <paramref name="name"/> (including the currently bound character's
|
||||
/// own file even if it has never been saved before), leaving every
|
||||
/// other row and every other table untouched. The live value has
|
||||
/// already been applied by the caller (<c>SetMetaOption</c>) before
|
||||
/// this runs; this only persists that same value into every profile
|
||||
/// file. Round 3 item 6: when a file's Settings table has no row for
|
||||
/// this name at all, <c>bk.a</c> APPENDS a new one
|
||||
/// (<c>bk.cs:22-27</c>: <c>cw3[0]=name, cw3[1]=value</c>, every other
|
||||
/// column left at its default-constructed <c>gy</c> — a "void" cell,
|
||||
/// <c>gy.cs:98-101</c>) rather than skipping that file, and the caller
|
||||
/// reports <c>files.Length</c> (every <c>.usd</c> file scanned,
|
||||
/// regardless of whether that file's own row already existed) —
|
||||
/// <see cref="SetOptionInAll"/> returns that same file count, not a
|
||||
/// per-row "successfully changed" tally.
|
||||
/// </summary>
|
||||
public int SetOptionInAll(string name, VtankSettingsProfileSerializer.AllSettings current)
|
||||
{
|
||||
|
|
@ -370,7 +379,6 @@ internal sealed class MossTankProfileStore
|
|||
? DynamicSettingDocument.From(liveDynamicValue)
|
||||
: null;
|
||||
|
||||
int updated = 0;
|
||||
foreach (string fileName in fileNames)
|
||||
{
|
||||
string? text = ReadUsdText(fileName);
|
||||
|
|
@ -395,7 +403,16 @@ internal sealed class MossTankProfileStore
|
|||
candidate.Cells[nameColumn].AsString().Equals(
|
||||
canonical, StringComparison.OrdinalIgnoreCase));
|
||||
if (row is null)
|
||||
continue;
|
||||
{
|
||||
// bk.cs:22-27: append a brand-new row rather than skip this
|
||||
// file. Every column other than [0]=name/[1]=value stays a
|
||||
// void gy() cell (gy.cs:98-101 writes only "0" for one).
|
||||
row = new VtankRow();
|
||||
for (int column = 0; column < table.ColumnNames.Count; column++)
|
||||
row.Cells.Add(new VtankCell { Tag = "0" });
|
||||
row.Cells[nameColumn] = VtankCell.String(canonical);
|
||||
table.Rows.Add(row);
|
||||
}
|
||||
row.Cells[valueColumn] = captured;
|
||||
WriteUsdText(fileName, database.Render());
|
||||
if (fileName.Equals(_currentDatabaseFileName, StringComparison.Ordinal))
|
||||
|
|
@ -407,9 +424,11 @@ internal sealed class MossTankProfileStore
|
|||
sidecar.CombatDynamicSettings[canonical] = dynamicDocument;
|
||||
WriteJson(SideCarKey(fileName), sidecar);
|
||||
}
|
||||
updated++;
|
||||
}
|
||||
return updated;
|
||||
// bk.cs:32: PluginCore.a(... + files.Length + " profiles)") reports
|
||||
// the TOTAL file count scanned, not a per-file "did it actually
|
||||
// change" tally.
|
||||
return fileNames.Count;
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -1517,6 +1517,97 @@ public sealed class MossTankPanelTests
|
|||
"uboptget['AttackDistance']").AsNumber(), precision: 7);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Round 3 item 6: retail's bk.a text (refs/vtank/decompiled/bk.cs:32)
|
||||
/// is "Done saving setting X to all profiles. (Changed N profiles)" —
|
||||
/// no "= value" suffix, unlike the plain "set" success message.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void VtankSetInAllReportsRetailsExactMessageText()
|
||||
{
|
||||
var automation = new FakeAutomation();
|
||||
var panel = new MossTankPanel(new FakeHost(automation));
|
||||
|
||||
Command(panel, "opt setinall AttackDistance 0.03");
|
||||
|
||||
string message = Assert.Single(automation.Messages, static text =>
|
||||
text.StartsWith("Done saving setting", StringComparison.Ordinal));
|
||||
Assert.StartsWith(
|
||||
"Done saving setting AttackDistance to all profiles. (Changed ",
|
||||
message,
|
||||
StringComparison.Ordinal);
|
||||
Assert.EndsWith(" profiles)", message, StringComparison.Ordinal);
|
||||
Assert.DoesNotContain("=", message, StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Round 3 item 6: a value that does not parse as the catalog's own
|
||||
/// declared type (EnableLooting is Bool) fails with retail's exact
|
||||
/// text (refs/vtank/decompiled/uTank2/PluginCore.cs:5501,5508,5612).
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData("set")]
|
||||
[InlineData("setinall")]
|
||||
public void VtankOptSetRejectsAWrongTypedValueWithRetailsExactText(string operation)
|
||||
{
|
||||
var automation = new FakeAutomation();
|
||||
var panel = new MossTankPanel(new FakeHost(automation));
|
||||
|
||||
Command(panel, $"opt {operation} EnableLooting notaboolean");
|
||||
|
||||
Assert.Equal(
|
||||
"Option set: Invalid value specified. Proper type of EnableLooting is System.Boolean.",
|
||||
Assert.Single(automation.Messages));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Round 3 item 6: <c>bk.a</c> (refs/vtank/decompiled/bk.cs:22-27)
|
||||
/// APPENDS a new Settings row — [0]=name, [1]=value, every other column
|
||||
/// left as a void <c>gy()</c> cell — when a <c>.usd</c> file's Settings
|
||||
/// table has no row for the option at all, rather than skipping that
|
||||
/// file outright.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void SetOptionInAllAppendsARowWhenAFileHasNoneForThatSetting()
|
||||
{
|
||||
var storage = new MemoryStorage();
|
||||
var automation = new FakeAutomation { Name = "Barris" };
|
||||
// A hand-trimmed Settings table (4 real columns: Setting, Value,
|
||||
// Description, SettingType) with exactly one row, for a DIFFERENT
|
||||
// setting — "EnableLooting" is deliberately absent.
|
||||
const string minimalUsd =
|
||||
"1\r\nSettings\r\n4\r\nSetting\r\nValue\r\nDescription\r\nSettingType\r\n"
|
||||
+ "y\r\nn\r\nn\r\nn\r\n1\r\ns\r\nEnableNav\r\nb\r\nFalse\r\ns\r\n\r\ni\r\n1\r\n";
|
||||
storage.Text["Other.usd"] = minimalUsd;
|
||||
|
||||
var store = new MossTankProfileStore(new FakeHost(automation, storage));
|
||||
var settings = new VtankSettingsProfileSerializer.AllSettings
|
||||
{
|
||||
Combat = new CombatSettings(),
|
||||
Buffs = new BuffSettings(),
|
||||
Vitals = new VitalSettings(),
|
||||
Inventory = new InventorySettings(),
|
||||
Navigation = new NavigationSettings(),
|
||||
};
|
||||
settings.Inventory.Loot.Enabled = true;
|
||||
|
||||
int count = store.SetOptionInAll("EnableLooting", settings);
|
||||
|
||||
VtankDatabase rewritten = VtankDatabase.Parse(storage.Text["Other.usd"]);
|
||||
VtankTable table = rewritten.Find("Settings")!;
|
||||
int nameColumn = table.ColumnIndex("Setting");
|
||||
int valueColumn = table.ColumnIndex("Value");
|
||||
VtankRow appended = Assert.Single(table.Rows, row =>
|
||||
row.Cells[nameColumn].AsString().Equals(
|
||||
"EnableLooting", StringComparison.OrdinalIgnoreCase));
|
||||
Assert.True(appended.Cells[valueColumn].AsBool());
|
||||
// The remaining columns (Description/SettingType) stay void, never
|
||||
// copied from the existing EnableNav row.
|
||||
Assert.Equal("0", appended.Cells[2].Tag);
|
||||
Assert.Equal("0", appended.Cells[3].Tag);
|
||||
Assert.True(count >= 1);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void VtankHelpAndExpressionsUseTheLocalCommandSurface()
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue