Spoonail-Iroiro / maginai

Mod loader
MIT License
1 stars 2 forks source link

`saveLoading`イベントを追加する #59

Closed Spoonail-Iroiro closed 2 months ago

Spoonail-Iroiro commented 3 months ago

セーブデータが選択されて、各ゲームオブジェクトへのデータのセットが始まる前のイベントsaveLoadingを追加する。 saveLoadingイベントではMod用セーブオブジェクトへのアクセスができる。

理由: maginai.modSaveでのMod用セーブオブジェクトの取得をsaveLoadedイベントで行うと、イベントより前の処理をセーブデータに保存された値に基づいて行えないため。 たとえばtGameRoutineMap.viewMapsaveLoadedイベントより前に呼ばれるため、tGameRoutineMap.viewMapにメソッドパッチした場合、そのメソッドはsaveLoadedイベントが起こるより前に呼ばれてしまう。

saveLoadingイベントとsaveLoadedイベントの使い分け・原則は以下の通り

考慮事項

soeklgb commented 3 months ago

modSaveの取得の仕方ですが、maginai.modSaveで取得するより以下のように引数で明示的に渡すのが良いと思います。

maginai.events.saveLoading.addHandler((modSave) => {
  modSave.getSaveObject("mod");
};

maginai.events.saveObjectRequired.addHandler((modSave) => {
  modSave.setSaveObject("mod", save);
};

maginaiからの取得だと個々のセーブデータとは別に保存できるように感じるかもしれません。

個人的にはこんなインターフェースが好みです。

maginai.modSave.load(modName, (modData) => {
});

maginai.modSave.save(modName, () => {
  return modData;
});
Spoonail-Iroiro commented 3 months ago

いっそmodSaveにはこのくらいのヘルパー関数addSaveHandlersがあってもいいかもしれない

let myModValue = 0;
const createSaveObject = () => ({myModValue});
const loadSaveObject = (modSaveObject) => myModValue = modData.myModValue;
maginai.modSave.addSaveHandlers(
  'myMod',
  createSaveObject,
  loadSaveObject
);

このaddSaveHandlersの呼び出しは下記のコードと同じ

maginai.events.saveLoading.addHandler(() => {
  const modSaveObject = maginai.modSave.getSaveObject('myMod');
  loadSaveObject(modSaveObject);
};

maginai.events.saveObjectRequired.addHandler(() => {
  const modSaveObject = createSaveObject();
  maginai.modSave.setSaveObject('myMod', modSaveObject);
};
Spoonail-Iroiro commented 3 months ago

下段のコードの通り、イベントsaveLoading/saveObjectRequiredの方はイベント引数なしでいきたい 理由: 個人的にはイベントハンドラでmodSaveサブモジュールを引数で受け取れてもあまりわかりやすくならないように見えるため。 enzaki氏の2番目の例のように直接Mod用セーブオブジェクトを受け取れればわかりやすいが、そのためにはmaginai側はaddHandler時点でMod名を受け取る必要がありaddHandlerのインターフェースにそぐわない。

Spoonail-Iroiro commented 3 months ago

補足: modSave.addSaveHandlersはほぼenzaki氏のsave/loadを1つのメソッドにまとめたメソッドだが、変更点と変更理由は以下の通り。

■2つあるメソッドを1つに統合 結局このmodSave.addSaveHandlersがhigh-level APIでmodSave.getSaveObject/modSave.setSaveObject/events.saveLoading/events.saveObjectRequiredがlow-level APIという関係になるので modSave.addSaveHandlersは便利な、ワンストップなメソッドとしていいのではないか、という理由。

■名前をハンドラー登録ということをわかりやすく save/loadだとどうしてもすぐセーブ・ロードを実行するように見えるのと、個別のセーブデータがセーブ/ロードされるたびにハンドラーが呼び出されるように見えにくいため。

soeklgb commented 3 months ago

わたしの出した最初の例ですが、modSavesaveLoadingsaveObjectRequiredの外で使う理由がないことをAPIレベルで表現したほうがよいのでは、という意図でした。(javascriptなのでルールに違反することは簡単ですが)言葉足らずですみません。

soeklgb commented 3 months ago
maginai.modSave.load(modName, (modData) => {
});

maginai.modSave.save(modName, () => {
  return modData;
});

こっちの例は他のModのセーブデータにアクセスしてはいけない、というのをAPIで表現したものです。

maginai.events.saveLoading.addHandler(() => {
  const modSaveObject = maginai.modSave.getSaveObject('myMod');
  loadSaveObject(modSaveObject);
};

maginai.events.saveObjectRequired.addHandler(() => {
  const modSaveObject = createSaveObject();
  maginai.modSave.setSaveObject('myMod', modSaveObject);
};

これだとドキュメントを読み込まないと正しいAPIの使い方ができないのでは?と思います。 events.saveLoadingmodSave.getSaveObject以外を目的に使われて、modSave.getSaveObjectevents.saveLoading以外で使われることになります。 たとえlow-level APIであれど意図しない使い方を可能にする理由はありません。

low-level APIで簡単に他のModのセーブデータにアクセスできるのならmodSave.addSaveHandlersはなくても良いでしょう。

save/loadだとどうしてもすぐセーブ・ロードを実行するように見えるのと、個別のセーブデータがセーブ/ロードされるたびにハンドラーが呼び出されるように見えにくいため。

save/loadの命名に関してはその通りだと思います。

Spoonail-Iroiro commented 2 months ago

他のModのセーブデータにアクセス

これについてはあるModをさらに拡張するMod等でありうるユースケースだとは思ってる

Spoonail-Iroiro commented 2 months ago

これらの、Mod用セーブデータの操作インターフェースについて 最終的にはどういう形がいいと思ってます?

soeklgb commented 2 months ago
maginai.modSave.addLoadHandler(modName, (modData) => {
});

maginai.modSave.addSaveHandler(modName, () => {
  return modData;
});

このようなsave/loadをハンドラだと分かる命名にしたものがいいと思ってますが、既存のハンドラのインターフェースにそぐわないと言われたらそうですね。

Modのセーブデータにいつでもどこからでも触れるようにするのならmaginai.events.saveLoading.addHandler/maginai.modSave.getSaveObjectのほうがいいと思います。

Spoonail-Iroiro commented 2 months ago

いっそmodSaveにはこのくらいのヘルパー関数addSaveHandlersがあってもいいかもしれない

let myModValue = 0;
const createSaveObject = () => ({myModValue});
const loadSaveObject = (modSaveObject) => myModValue = modData.myModValue;
maginai.modSave.addSaveHandlers(
  'myMod',
  createSaveObject,
  loadSaveObject
);

このaddSaveHandlersの呼び出しは下記のコードと同じ

maginai.events.saveLoading.addHandler(() => {
  const modSaveObject = maginai.modSave.getSaveObject('myMod');
  loadSaveObject(modSaveObject);
};

maginai.events.saveObjectRequired.addHandler(() => {
  const modSaveObject = createSaveObject();
  maginai.modSave.setSaveObject('myMod', modSaveObject);
};

これで実装したい ただしaddSaveHandlersは少し変更

let myModValue = 0;
const createSaveObject = () => ({myModValue});
const loadSaveObject = (modSaveObject, isNewGame) => {
  if (isNewGame) {
    myModValue=1;
  }
  else {
    myModValue = modSaveObject?.myModValue ?? 2;
  }
};
maginai.modSave.addSaveObjectHandlers(
  'myMod',
  createSaveObject,
  loadSaveObject
);
soeklgb commented 2 months ago

一応確認です。実装するのはこの三つですか?

Spoonail-Iroiro commented 2 months ago

この2つです。

maginai.events.saveObjectRequiredはすでに存在していますね。

Spoonail-Iroiro commented 2 months ago

今更ながらaddSaveObjectHandlersで「ロード時にnameに対応するセーブオブジェクトが存在しない」ときのhandlerは別にしたほうがいいのではとなった

let myModValue = 0;
const initializeWithoutSaveObject = (isNewGame) => {
  myModValue = isNewGame ? 1 : 2;
}
const loadSaveObject = (modSaveObject) => {
  myModValue = modSaveObject.myModValue;
};
const createSaveObject = () => ({myModValue});
maginai.modSave.addSaveObjectHandlers(
  'myMod',
  initializeWithoutSaveObject,
  loadSaveObject,
  createSaveObject,
);

これは以下と同じ

maginai.events.saveLoading.addHandler((e) => {
  const modSaveObject = maginai.modSave.getSaveObject('myMod');
  if (modSaveObject === undefined) {
    initializeWithoutSaveObject(e.isNewGame);
  }
  else {
    loadSaveObject(modSaveObject);
  }
};

maginai.events.saveObjectRequired.addHandler(() => {
  const modSaveObject = createSaveObject();
  maginai.modSave.setSaveObject('myMod', modSaveObject);
};

参考:https://github.com/limoka/DSP-Mods/blob/master/Mods/DSPModSave/src/DSPModSave/IModCanSave.cs