-
-
Notifications
You must be signed in to change notification settings - Fork 1
feat: 汎用プラグインを追加 #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: 汎用プラグインを追加 #20
Conversation
White-Green
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Webフロントっぽいところは見てません
| /// # Panics | ||
| /// | ||
| /// プラグインが初期化されていない場合や、二重に呼び出された場合にパニックします。 | ||
| fn with_instance<R>(f: impl FnOnce(&Self) -> R) -> R |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[inline]つけないとインライン展開されないかも
詳しく知らないので嘘かも
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
実はいつinlineするべきかがわかっていない
| /// - `key`にヌル文字が含まれている場合、失敗します。 | ||
| /// - `data` の長さが保存されているデータの長さと一致しない場合、失敗します。 | ||
| /// - 指定されたキーに対応するデータが存在しない場合、失敗します。 | ||
| pub fn get_param_binary(&self, key: &str, data: &mut [u8]) -> AnyResult<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
長さ一致は厳しい制約だなぁとおもったがSDK側がそういう仕様なのか(バッファのほうが長い分にはOKで、読み込まれたバイト数を返すAPIのほうがよく見る)
悩ましい...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なんと、そういう仕様(どうして...)
| } | ||
|
|
||
| /// オブジェクトを削除します。 | ||
| pub fn delete_object(&self, object: &ObjectHandle) -> AnyResult<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
引数&ObjectHandleじゃなくてObjectHandleにしたくないですか
同様に更新系は&mut ObjectHandleをとるとか
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
実はオブジェクトを削除してもObjectHandleは有効(具体的にはundo)なので、やめた
mutのほうが良さそうというのはそう、というかSync切ってはいるけどmut selfにしたほうが良いかも
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&mut selfにしたらsections.layers()とlayer.objects()で回すパターンができなくなってちょっと頭を抱えている(!SyncではあるのでUBは起きないはずだがそれはそれとして普通にMutableが複数存在しているのは良くない...)
This comment was marked as duplicate.
This comment was marked as duplicate.
|
プレビューがデプロイされました: 更新時点でのコミット: 6450ce5 |
No description provided.