fix

sunfish-shogi · Oct 20, 2024 · 095ad00 · 095ad00
1 parent ef1a4d7
commit 095ad00
Show file tree

Hide file tree

Showing 7 changed files with 150 additions and 95 deletions.
diff --git a/src/common/settings/usi.ts b/src/common/settings/usi.ts
@@ -197,6 +197,57 @@ function isValidOptionValue(option: USIEngineOption): boolean {
   return true;
 }
 
+export type USIEngineOptionDiff = {
+  name: string;
+  leftValue: string | number | undefined;
+  rightValue: string | number | undefined;
+  mergeable: boolean;
+};
+
+export function compareUSIEngineOptions(left: USIEngine, right: USIEngine): USIEngineOptionDiff[] {
+  const result = [] as USIEngineOptionDiff[];
+  function append(leftOption?: USIEngineOption, rightOption?: USIEngineOption) {
+    const leftHasValue = leftOption && leftOption.type !== "button";
+    const rightHasValue = rightOption && rightOption.type !== "button";
+    if (leftHasValue && rightHasValue && leftOption.value !== rightOption.value) {
+      result.push({
+        name: leftOption.name,
+        leftValue: getUSIEngineOptionCurrentValue(leftOption),
+        rightValue: getUSIEngineOptionCurrentValue(rightOption),
+        mergeable: leftOption.type === rightOption.type,
+      });
+    } else if (leftHasValue && !rightHasValue) {
+      result.push({
+        name: leftOption.name,
+        leftValue: getUSIEngineOptionCurrentValue(leftOption),
+        rightValue: undefined,
+        mergeable: false,
+      });
+    } else if (!leftHasValue && rightHasValue) {
+      result.push({
+        name: rightOption.name,
+        leftValue: undefined,
+        rightValue: getUSIEngineOptionCurrentValue(rightOption),
+        mergeable: false,
+      });
+    }
+  }
+  function compareOrder(a: USIEngineOption, b: USIEngineOption): number {
+    return a.order < b.order ? -1 : 1;
+  }
+  for (const leftOption of Object.values(left.options).sort(compareOrder)) {
+    const rightOption = right.options[leftOption.name];
+    append(leftOption, rightOption);
+  }
+  for (const rightOption of Object.values(right.options).sort(compareOrder)) {
+    const leftOption = left.options[rightOption.name];
+    if (!leftOption) {
+      append(undefined, rightOption);
+    }
+  }
+  return result;
+}
+
 export interface ImmutableUSIEngines {
   hasEngine(uri: string): boolean;
   getEngine(uri: string): USIEngine | undefined;

diff --git a/src/renderer/view/dialog/USIEngineMergeDialog.vue b/src/renderer/view/dialog/USIEngineMergeDialog.vue
@@ -34,19 +34,37 @@
               <span v-if="option.displayName">)</span>
             </div>
             <div class="row">
-              <div class="engine-column">{{ option.leftValue }}</div>
-              <div class="engine-column">{{ option.rightValue }}</div>
+              <div class="engine-column">
+                <input
+                  v-if="option.leftValue"
+                  class="option-value"
+                  readonly
+                  :value="option.leftValue"
+                />
+              </div>
+              <div class="engine-column">
+                <input
+                  v-if="option.rightValue"
+                  class="option-value"
+                  readonly
+                  :value="option.rightValue"
+                />
+              </div>
             </div>
             <div v-if="!option.mergeable">{{ t.thisItemCannotBeMerged }}</div>
-            <div v-else class="row controls">
-              <button @click="mergeToLeft(option.name)">
-                <Icon :icon="IconType.BACK" />
-                {{ t.mergeToLeft }}
-              </button>
-              <button @click="mergeToRight(option.name)">
-                {{ t.mergeToRight }}
-                <Icon :icon="IconType.NEXT" />
-              </button>
+            <div v-else class="row">
+              <div class="engine-column">
+                <button @click="mergeToLeft(option.name)">
+                  <Icon :icon="IconType.BACK" />
+                  {{ t.mergeToLeft }}
+                </button>
+              </div>
+              <div class="engine-column">
+                <button @click="mergeToRight(option.name)">
+                  {{ t.mergeToRight }}
+                  <Icon :icon="IconType.NEXT" />
+                </button>
+              </div>
             </div>
           </div>
         </div>
@@ -66,11 +84,7 @@
 <script setup lang="ts">
 import { computed, onBeforeUnmount, onMounted, PropType, reactive, ref } from "vue";
 import PlayerSelector from "./PlayerSelector.vue";
-import {
-  getUSIEngineOptionCurrentValue as getCurrentValue,
-  ImmutableUSIEngines,
-  USIEngineOption,
-} from "@/common/settings/usi";
+import { compareUSIEngineOptions, ImmutableUSIEngines } from "@/common/settings/usi";
 import { showModalDialog } from "@/renderer/helpers/dialog";
 import { installHotKeyForDialog, uninstallHotKeyForDialog } from "@/renderer/devices/hotkey";
 import { t, usiOptionNameMap } from "@/common/i18n";
@@ -131,72 +145,19 @@ const mergeToRight = (name: string) => {
   right.options[name] = left.options[name];
 };
 
-type DiffItem = {
-  name: string;
-  displayName: string;
-  leftValue: string;
-  rightValue: string;
-  mergeable: boolean;
-  order: number;
-};
-
 const diff = computed(() => {
   const left = usiEngines.getEngine(leftEngineURI.value);
   const right = usiEngines.getEngine(rightEngineURI.value);
   if (!left || !right) {
     return [];
   }
-  const leftOptions = left.options;
-  const rightOptions = right.options;
-
-  const result = [] as DiffItem[];
   const translate = appSettings.translateEngineOptionName;
-  function append(leftOption?: USIEngineOption, rightOption?: USIEngineOption) {
-    const leftOptionHasValue = leftOption && leftOption.type !== "button";
-    const rightOptionHasValue = rightOption && rightOption.type !== "button";
-    const leftValue = leftOptionHasValue ? String(getCurrentValue(leftOption)) : "";
-    const rightValue = rightOptionHasValue ? String(getCurrentValue(rightOption)) : "";
-    if (leftOptionHasValue && rightOptionHasValue && leftOption.value !== rightOption.value) {
-      result.push({
-        name: leftOption.name,
-        displayName: translate ? usiOptionNameMap[leftOption.name] : "",
-        leftValue,
-        rightValue,
-        mergeable: true,
-        order: leftOption.order,
-      });
-    } else if (leftOptionHasValue && !rightOptionHasValue) {
-      result.push({
-        name: leftOption.name,
-        displayName: translate ? usiOptionNameMap[leftOption.name] : "",
-        leftValue,
-        rightValue: "",
-        mergeable: false,
-        order: leftOption.order,
-      });
-    } else if (!leftOptionHasValue && rightOptionHasValue) {
-      result.push({
-        name: rightOption.name,
-        displayName: translate ? usiOptionNameMap[rightOption.name] : "",
-        leftValue: "",
-        rightValue,
-        mergeable: false,
-        order: 10000 + rightOption.order,
-      });
-    }
-  }
-
-  for (const left of Object.values(leftOptions)) {
-    const right = rightOptions[left.name];
-    append(left, right);
-  }
-  for (const right of Object.values(rightOptions)) {
-    const left = leftOptions[right.name];
-    if (!left) {
-      append(undefined, right);
-    }
-  }
-  return result.sort((a, b) => (a.order < b.order ? -1 : 1));
+  return compareUSIEngineOptions(left, right).map((option) => {
+    return {
+      ...option,
+      displayName: translate ? usiOptionNameMap[option.name] : "",
+    };
+  });
 });
 
 const ok = () => {
@@ -221,14 +182,17 @@ const cancel = () => {
   overflow: auto;
 }
 .engine-column {
-  padding: 0px 5px;
-  width: 50%;
+  margin: 0 5px;
+  width: calc(50% - 10px);
 }
 .option {
-  padding: 5px 0px;
+  padding: 5px 0;
   border-bottom: 1px solid var(--text-separator-color);
 }
-.controls {
-  justify-content: center;
+.option > *:not(:last-child) {
+  margin-bottom: 5px;
+}
+.option-value {
+  width: 100%;
 }
 </style>
diff --git a/src/tests/background/usi/index.spec.ts b/src/tests/background/usi/index.spec.ts
@@ -13,7 +13,7 @@ import {
   setupPlayer,
 } from "@/background/usi";
 import { ChildProcess } from "@/background/usi/process";
-import { usiEngines } from "@/tests/mock/usi";
+import { testUSIEngine } from "@/tests/mock/usi";
 import { MockedClass } from "vitest";
 
 vi.mock("@/background/usi/process");
@@ -91,7 +91,7 @@ describe("background/usi/index", () => {
   });
 
   it("go", async () => {
-    const setupPromise = setupPlayer(usiEngines, 10);
+    const setupPromise = setupPlayer(testUSIEngine, 10);
     const onReceive = getChildProcessHandler("receive");
     const onClose = getChildProcessHandler("close");
     expect(mockChildProcess.prototype.send).lastCalledWith("usi");
@@ -139,7 +139,7 @@ describe("background/usi/index", () => {
   it("early-ponder", async () => {
     const setupPromise = setupPlayer(
       {
-        ...usiEngines,
+        ...testUSIEngine,
         enableEarlyPonder: true,
       },
       10,
@@ -169,7 +169,7 @@ describe("background/usi/index", () => {
   });
 
   it("activeSessionCount", async () => {
-    const setupPromise = setupPlayer(usiEngines, 10);
+    const setupPromise = setupPlayer(testUSIEngine, 10);
     const onReceive = getChildProcessHandler("receive");
     const onClose = getChildProcessHandler("close");
     expect(mockChildProcess.prototype.send).lastCalledWith("usi");

diff --git a/src/tests/common/settings/usi.spec.ts b/src/tests/common/settings/usi.spec.ts
@@ -8,7 +8,9 @@ import {
   USIEngine,
   USIEngines,
   validateUSIEngine,
+  compareUSIEngineOptions,
 } from "@/common/settings/usi";
+import { testUSIEngine } from "@/tests/mock/usi";
 
 describe("settings/usi", () => {
   it("getUSIEngineOptionCurrentValue", () => {
@@ -356,6 +358,44 @@ describe("settings/usi", () => {
     });
   });
 
+  it("compareUSIEngineOptions", () => {
+    const lhs: USIEngine = {
+      ...testUSIEngine,
+      options: {
+        USI_Hash: { name: "USI_Hash", type: "spin", order: 0, default: 32, value: 64 },
+        USI_Ponder: { name: "USI_Ponder", type: "check", order: 1, default: "true" },
+        Refresh: { name: "Refresh", type: "button", order: 2 },
+        Log: { name: "Log", type: "button", order: 3 },
+        Book: { name: "Book", type: "filename", order: 4, default: "<empty>", value: "book.db" },
+        StringA: { name: "StringA", type: "string", order: 4, value: "foo" },
+        StringB: { name: "StringB", type: "string", order: 5, value: "bar" },
+        Depth: { name: "Depth", type: "spin", order: 6, default: 1, value: 2 }, // only lhs
+      },
+    };
+    const rhs: USIEngine = {
+      ...testUSIEngine,
+      options: {
+        USI_Hash: { name: "USI_Hash", type: "spin", order: 0, default: 32 }, // no value
+        USI_Ponder: { name: "USI_Ponder", type: "check", order: 1, default: "true" }, // equal
+        Refresh: { name: "Refresh", type: "button", order: 2 }, // skip
+        Log: { name: "Log", type: "check", order: 3, value: "true" }, // different type
+        Book: { name: "Book", type: "combo", order: 4, vars: [], value: "standard.db" }, // different type
+        StringA: { name: "StringA", type: "string", order: 4, value: "foo" }, // equal
+        StringB: { name: "StringB", type: "string", order: 5, value: "baz" }, // different value
+        ResignValue: { name: "ResignValue", type: "spin", order: 6, default: 1, value: -3000 }, // only rhs
+      },
+    };
+    const result = compareUSIEngineOptions(lhs, rhs);
+    expect(result).toStrictEqual([
+      { name: "USI_Hash", leftValue: 64, rightValue: 32, mergeable: true },
+      { name: "Log", leftValue: undefined, rightValue: "true", mergeable: false },
+      { name: "Book", leftValue: "book.db", rightValue: "standard.db", mergeable: false },
+      { name: "StringB", leftValue: "bar", rightValue: "baz", mergeable: true },
+      { name: "Depth", leftValue: 2, rightValue: undefined, mergeable: false },
+      { name: "ResignValue", leftValue: undefined, rightValue: -3000, mergeable: false },
+    ]);
+  });
+
   it("USIEngines", () => {
     const engines = new USIEngines(
       JSON.stringify({

diff --git a/src/tests/mock/research.ts b/src/tests/mock/research.ts
@@ -1,21 +1,21 @@
 import { ResearchSettings } from "@/common/settings/research";
-import { usiEngines } from "./usi";
+import { testUSIEngine } from "./usi";
 
 export const researchSettings: ResearchSettings = {
-  usi: usiEngines,
+  usi: testUSIEngine,
   enableMaxSeconds: false,
   maxSeconds: 5,
 };
 
 export const researchSettingsMax5Seconds: ResearchSettings = {
-  usi: usiEngines,
+  usi: testUSIEngine,
   enableMaxSeconds: true,
   maxSeconds: 5,
 };
 
 export const researchSettingsSecondaryEngines: ResearchSettings = {
-  usi: usiEngines,
-  secondaries: [{ usi: usiEngines }, { usi: usiEngines }],
+  usi: testUSIEngine,
+  secondaries: [{ usi: testUSIEngine }, { usi: testUSIEngine }],
   enableMaxSeconds: false,
   maxSeconds: 5,
 };
diff --git a/src/tests/mock/usi.ts b/src/tests/mock/usi.ts
@@ -1,6 +1,6 @@
 import { USIEngine } from "@/common/settings/usi";
 
-export const usiEngines: USIEngine = {
+export const testUSIEngine: USIEngine = {
   uri: "es://usi/test-engine",
   name: "my usi engine",
   defaultName: "engine",
@@ -11,7 +11,7 @@ export const usiEngines: USIEngine = {
   enableEarlyPonder: false,
 };
 
-export const usiEnginesWithPonder: USIEngine = {
+export const testUSIEngineWithPonder: USIEngine = {
   uri: "es://usi/test-engine",
   name: "my usi engine",
   defaultName: "engine",

diff --git a/src/tests/renderer/players/usi.spec.ts b/src/tests/renderer/players/usi.spec.ts
@@ -1,7 +1,7 @@
 import api, { API } from "@/renderer/ipc/api";
 import { onUSIBestMove, onUSIInfo, USIPlayer } from "@/renderer/players/usi";
 import { Move, parsePV, Record } from "tsshogi";
-import { usiEngines, usiEnginesWithPonder } from "@/tests/mock/usi";
+import { testUSIEngine, testUSIEngineWithPonder } from "@/tests/mock/usi";
 import { Mocked } from "vitest";
 
 vi.mock("@/renderer/ipc/api");
@@ -38,7 +38,7 @@ describe("usi", () => {
     const record1 = Record.newByUSI(usi1) as Record;
     const record2 = Record.newByUSI(usi2) as Record;
     const record3 = Record.newByUSI(usi3) as Record;
-    const player = new USIPlayer(usiEnginesWithPonder, 10);
+    const player = new USIPlayer(testUSIEngineWithPonder, 10);
     try {
       await player.launch();
       const searchHandler = {
@@ -101,7 +101,7 @@ describe("usi", () => {
     const usi2 = "position startpos moves 7g7f 3c3d 2g2f";
     const record1 = Record.newByUSI(usi1) as Record;
     const record2 = Record.newByUSI(usi2) as Record;
-    const player = new USIPlayer(usiEnginesWithPonder, 10);
+    const player = new USIPlayer(testUSIEngineWithPonder, 10);
     try {
       await player.launch();
       const searchHandler = {
@@ -128,7 +128,7 @@ describe("usi", () => {
     const usi = "position startpos moves 7g7f 3c3d";
     const record = Record.newByUSI(usi) as Record;
     const onSearchInfo = vi.fn();
-    const player = new USIPlayer(usiEngines, 10, onSearchInfo);
+    const player = new USIPlayer(testUSIEngine, 10, onSearchInfo);
     try {
       await player.launch();
       const searchHandler = {