fix:Addressable登録時のaddress名を変更 #491
Conversation
|
""" WalkthroughDynamicタイルのエクスポート処理がリファクタリングされ、アドレス命名、タイル管理、コードの明確化が行われました。タイルマネージャのシングルトン化、アドレス名の一意性確保、親Transformの管理、エラーハンドリングの強化などが実装されています。 Changes
Sequence Diagram(s)sequenceDiagram
participant Exporter as DynamicTileExporter
participant Manager as PLATEAUTileManager
participant Tile as PLATEAUDynamicTile
Exporter->>Manager: GetOrCreateTileManager()
Exporter->>Manager: Clear()
loop 各CityObjectGroup
Exporter->>Exporter: GetAddress(GameObject)
Exporter->>Manager: Check address uniqueness
Exporter->>Tile: new PLATEAUDynamicTile(address, parent)
Exporter->>Manager: AddTile(tile)
Exporter->>Exporter: Save prefab, register addressable
end
Exporter->>Manager: Save catalog path
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors how Addressable names are generated by extracting mesh codes from parent GML objects and integrates a PLATEAUTileManager to track dynamic tiles.
- Expose
DynamicTilesonPLATEAUTileManagerand extendPLATEAUDynamicTileto include its parent transform - Update
DynamicTileExporterto compute and dedupe address names via a newGetAddressmethod, then register tiles with the manager - Add helper
GetOrCreateTileManagerand detailed address‐generation logic
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Runtime/DynamicTile/PLATEAUTileManager.cs | Added public DynamicTiles property |
| Runtime/DynamicTile/PLATEAUDynamicTile.cs | Updated constructor to accept a parent parameter |
| Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs | Compute address names, handle duplicates, integrate manager, and add helper methods |
Comments suppressed due to low confidence (5)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs:45
- [nitpick] The name
isExcludeAssetFolderis counterintuitive (it’s true when a build path is provided). Consider renaming to something likehasBuildFolderPathoruseSeparateAssetFolderfor clarity.
var isExcludeAssetFolder = !string.IsNullOrEmpty(buildFolderPath);
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs:38
- [nitpick] This error message mixes English and Japanese and may not clearly communicate to all users. Consider unifying the language and referencing the UI term "都市モデル" consistently.
onError?.Invoke("CityObjectGroupが見つかりません。都市モデルにCityObjectGroupが含まれていることを確認してください。");
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs:226
- The new
GetAddresslogic has multiple branches (2nd vs. 3rd mesh, invalid codes, path splits). Consider adding unit tests covering each branch to ensure address names stay correct over time.
private static string GetAddress(GameObject obj)
Runtime/DynamicTile/PLATEAUDynamicTile.cs:29
- The class assigns
this.parent = parentbut I don’t see aparentfield or property declared—add aprivate Transform parent;(or an exposed property) to avoid a compilation error.
public PLATEAUDynamicTile(string address, Transform parent)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs:171
- The comment claims the method may return null, but the implementation always returns a non-null manager. Update or remove the null clause to keep docs accurate.
/// <returns>PLATEAUTileManagerのインスタンス。生成に失敗した場合はnull。</returns>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs (3)
84-93: アドレス名の重複処理は適切ですが、コメントの修正が必要です重複チェックと連番付与のロジックは正しく実装されていますが、90行目のコメントが実装と一致していません。
- // 最後の部分（baseName）に連番を付ける + // アドレス名全体に連番を付ける
275-277: ズームレベルの取得方法について確認が必要です現在ズームレベルは0にハードコードされています。TODOコメントがありますが、実装計画について確認させてください。
ズームレベルの取得ロジックの実装について、新しいissueを作成しますか？
221-227: GetAddressメソッドのドキュメントを詳細化することを推奨しますメソッドの動作をより明確にするため、ドキュメントコメントを拡張することを提案します。
/// <summary> /// 指定したGameObjectからAddressables用のアドレス名を取得します。 + /// 親のGMLオブジェクトからメッシュコードを抽出し、以下の形式でアドレス名を生成します： + /// - 2次メッシュ（6桁）: tile_{meshCode}__{baseName} + /// - 3次メッシュ（8桁）: tile_zoom_{zoomLevel}_grid_{meshCode}_{baseName} /// </summary> /// <param name="obj">アドレスを取得したいGameObject</param> - /// <returns>アドレス名。取得できない場合は空文字列。</returns> + /// <returns>生成されたアドレス名。メッシュコードが無効な場合は空文字列。</returns>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs(8 hunks)Runtime/DynamicTile/PLATEAUDynamicTile.cs(1 hunks)Runtime/DynamicTile/PLATEAUTileManager.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs (4)
Runtime/DynamicTile/PLATEAUTileManager.cs (4)
ClearTiles(144-147)AddTile(125-139)SaveCatalogPath(47-51)PLATEAUTileManager(10-148)Runtime/CityAdjust/ConvertToAsset/ConvertToAsset.cs (1)
List(45-156)Runtime/DynamicTile/PLATEAUDynamicTile.cs (1)
PLATEAUDynamicTile(29-33)Runtime/Util/Dialogue.cs (1)
Dialogue(13-37)
🔇 Additional comments (7)
Runtime/DynamicTile/PLATEAUTileManager.cs (1)
14-14: LGTM!
dynamicTilesリストへの読み取り専用アクセスを提供する適切な実装です。外部からの直接的な変更を防ぎつつ、必要な情報へのアクセスを可能にしています。Runtime/DynamicTile/PLATEAUDynamicTile.cs (1)
29-33: LGTM!タイルと親Transformの関係を管理するための適切な実装です。エクスポート処理で必要な情報を保持できるようになります。
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs (5)
3-3: 必要なusing文の追加
GridCodeクラスとLINQ操作のために必要な名前空間が適切に追加されています。Also applies to: 12-12
35-41: 変数名とエラーメッセージの改善
cityObjectGroupsという変数名は実際の型PLATEAUCityObjectGroupをより正確に反映しており、エラーメッセージも具体的になりました。
56-60: タイルマネージャーの適切な初期化シングルトンパターンでタイルマネージャーを取得し、処理開始時にタイルをクリアすることで、クリーンな状態から開始できます。
168-182: タイルマネージャーの取得・生成メソッドの適切な実装シングルトンパターンでタイルマネージャーを管理する良い実装です。既存のインスタンスがない場合のみ新規作成します。
260-265: GridCode検証の適切な実装メッシュコードの妥当性チェックが適切に実装されています。無効なコードの場合のエラーハンドリングも問題ありません。
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs (1)
269-271: 2次メッシュのアドレス形式がPR要件と異なります過去のレビューコメントでも指摘されていますが、PR要件では
tile_(secondary mesh)__(original game object name)_(ID if duplicate names exist)という形式（tile_の後にアンダースコア2つ）ですが、実装ではtile_{meshCode}_{baseName}（アンダースコア1つ）になっています。PR要件に合わせて修正してください：
- return $"tile_{meshCode}_{baseName}"; + return $"tile_{meshCode}__{baseName}";
🧹 Nitpick comments (1)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs (1)
275-276: ズームレベルの実装が必要ですTODOコメントで示されているように、現在ズームレベルがハードコードされています。PR要件の「tile_zoom_(tile zoom level)grid...」形式を満たすために、適切なズームレベルの取得処理が必要です。
ズームレベルの取得処理の実装について支援が必要でしたら、メッシュコードからズームレベルを算出するロジックの提案も可能です。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs(8 hunks)
🔇 Additional comments (7)
Editor/Window/Main/Tab/DynamicTileGUI/DynamicTileExporter.cs (7)
3-12: 必要なusing文の追加を承認
System.Linqの追加により、新しく実装されたアドレス名生成処理で必要なLINQメソッドが使用できるようになっています。
35-40: 変数名の改善とエラーメッセージの明確化を承認
cityObjectsからcityObjectGroupsへの変数名変更により、コードの可読性が向上し、エラーメッセージもより具体的になっています。
57-60: タイルマネージャーのシングルトン化を承認新しく追加された
GetOrCreateTileManager()メソッドにより、タイルマネージャーの取得と生成が適切に管理されています。処理開始前にタイルをクリアすることで、状態の一貫性が保たれています。
76-82: アドレス名生成処理の追加を承認新しく実装された
GetAddress()メソッドによる一意なアドレス名の生成処理が適切に統合されています。失敗時の警告ログと処理の継続も適切です。
84-93: アドレス名の重複回避処理を承認タイルマネージャーの既存タイルリストを使用した重複チェックにより、ファイルシステムへの依存を減らし、より効率的な重複回避が実現されています。
168-181: タイルマネージャー取得メソッドの実装を承認シンプルで明確な実装により、シングルトンパターンが適切に実装されています。既存のマネージャーがない場合の新規作成も適切です。
260-265: メッシュコードバリデーション処理を承認
GridCode.Create()を使用したメッシュコードの妥当性チェックにより、不正な形式の早期検出と適切なエラーハンドリングが実現されています。
関連リンク
https://synesthesias.atlassian.net/browse/CPSDK25-124
こちらの対応
実装内容
マージ前確認項目
動作確認
tile_zoom_(タイルのズームレベル)_grid_(タイルの位置を示すメッシュコード)_(従来のゲームオブジェクト名)_(同名の場合のID)tile_(2次メッシュ)_(従来のゲームオブジェクト名)_(同名の場合のID)その他
Summary by CodeRabbit
新機能
改善