feat: category-specific import menus & DAT parser independent feature flags#1
Conversation
…t menus Split the single "Merge Session Into Current" menu into 4 separate menus: - Import Items (cube icon, blue) - Import Outfits (shirt icon, green) - Import Effects (wand icon, yellow) - Import Missiles (arrow icon, pink) Each menu dynamically lists available session DAT files and imports only the selected category, making it easier to selectively merge assets. Changes: - MainWindow.axaml: 4 new MenuItems with icons and category colors - MainWindow.axaml.cs: WireImportMenu helper for dynamic submenu wiring - MainWindowViewModel.cs: MergeSessionAsync accepts ThingCategory filter
…ps as independent feature flags The DAT parser previously derived all three texture features (U32 sprites, enhanced animations, frame groups) from the protocol version number. This failed for custom DAT files (e.g. PStory) that use hybrid combinations like V4 flags (proto 854) with U32 sprites + enhanced animations + frame groups - a mix no standard Tibia version uses. Changes: - ParseThing now accepts separate bool parameters for each feature - Load() tries 8 feature flag combinations per protocol (like PStory client tryLoadDatWithFallbacks) and picks the first perfect parse - DatData model stores EnhancedAnimations and FrameGroups properties - Save/WriteThing respects feature flags for sprite size and animations This fixes Effects (2208) and Missiles (223) showing as 0 when loading PStory DAT files that require proto=854+ext+anim+fg.
There was a problem hiding this comment.
Pull request overview
Adds category-scoped session importing in the Assets & Map Editor UI and updates the DAT parser to treat extended, enhancedAnimations, and frameGroups as independent feature flags (with fallback attempts across combinations) to better support hybrid/custom DAT formats.
Changes:
- Split “Merge Session Into Current…” into four per-category import menus (Items/Outfits/Effects/Missiles) that import only the selected category.
- Updated DAT parsing to try multiple independent feature-flag combinations per protocol and preserve best partial parses when perfect parsing is not possible.
- Updated DAT writing to respect
extended(U16 vs U32 sprite indices) and the independent animation/frame-group flags.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/OTB/DatFile.cs | Implements independent feature-flag parsing fallbacks; propagates flags into DatData; updates DAT writing for sprite index size and optional enhanced animation/frame-groups. |
| src/App/ViewModels/MainWindowViewModel.cs | Adds optional category filter to MergeSessionAsync and exposes a category→dictionary selector used by import menus. |
| src/App/MainWindow.axaml.cs | Wires four dynamic import submenus that call the category-filtered import path. |
| src/App/MainWindow.axaml | Replaces the single merge menu entry with four category-specific import menu items and icons. |
Comments suppressed due to low confidence (1)
src/App/ViewModels/MainWindowViewModel.cs:5209
- Making GetCategoryDict public exposes the mutable backing dictionaries for DatData to any caller, which makes it easy for UI/code-behind to mutate session state unintentionally. If you only need counts for the import menu, consider keeping this private/internal and adding a small helper that returns the count (or returning an IReadOnlyDictionary) instead of the mutable Dictionary.
public static Dictionary<ushort, DatThingType> GetCategoryDict(DatData data, ThingCategory category)
{
return category switch
{
ThingCategory.Outfit => data.Outfits,
ThingCategory.Effect => data.Effects,
ThingCategory.Missile => data.Missiles,
_ => data.Items,
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Header = $"{source.Name} ({dict.Count} {category.ToString().ToLowerInvariant()}s)", | ||
| Tag = source, | ||
| }; | ||
| mi.Click += async (_, _) => await vm.MergeSessionAsync(source, category); |
There was a problem hiding this comment.
This Click handler is async and its exceptions will go unhandled (event handlers are effectively async void). Wrap the awaited call in a try/catch and route errors into StatusText/logging to avoid crashes if MergeSessionAsync throws.
| mi.Click += async (_, _) => await vm.MergeSessionAsync(source, category); | |
| mi.Click += async (_, _) => | |
| { | |
| try | |
| { | |
| await vm.MergeSessionAsync(source, category); | |
| } | |
| catch (Exception ex) | |
| { | |
| Console.Error.WriteLine($"Error merging session '{source.Name}' for category '{category}': {ex}"); | |
| } | |
| }; |
| int totalSource = categories.Sum(c => c.Item2.Count); | ||
| StatusText = $"Analyzing {totalSource} source things for duplicates…"; | ||
| var label = categoryFilter?.ToString().ToLowerInvariant() ?? "thing"; | ||
| StatusText = $"Analyzing {totalSource} source {label}s for duplicates…"; |
There was a problem hiding this comment.
StatusText pluralization is always "{label}s" which produces grammatically incorrect text for singular (e.g., "Analyzing 1 source items"). Consider using a simple singular/plural branch based on totalSource to keep status messages correct.
| StatusText = $"Analyzing {totalSource} source {label}s for duplicates…"; | |
| var labelText = totalSource == 1 ? label : label + "s"; | |
| StatusText = $"Analyzing {totalSource} source {labelText} for duplicates…"; |
| } | ||
|
|
||
| throw new InvalidOperationException( | ||
| $"Failed to parse {Path.GetFileName(path)} (sig=0x{sig:X8}, size={raw.Length}). No protocol/extended combination worked."); |
There was a problem hiding this comment.
The final failure message still says "No protocol/extended combination worked" but loading now varies multiple independent feature flags (extended/enhancedAnimations/frameGroups). Update the message to reflect that all protocol/feature-flag combinations were attempted so logs/errors are accurate.
| $"Failed to parse {Path.GetFileName(path)} (sig=0x{sig:X8}, size={raw.Length}). No protocol/extended combination worked."); | |
| $"Failed to parse {Path.GetFileName(path)} (sig=0x{sig:X8}, size={raw.Length}). No protocol/feature-flag combination (extended/enhancedAnimations/frameGroups) worked."); |
| if (isOutfit && frameGroups) | ||
| w.U8((byte)thing.FrameGroups.Length); | ||
|
|
||
| for (int g = 0; g < thing.FrameGroups.Length; g++) |
There was a problem hiding this comment.
When frameGroups is disabled, this method still iterates over thing.FrameGroups.Length but does not write the group count/type fields. If thing.FrameGroups has more than 1 entry, the saved DAT will be malformed. Consider writing only the first frame group (or throwing/normalizing) when frameGroups == false, and similarly validate that FrameGroups is non-empty when frameGroups == true.
| if (isOutfit && frameGroups) | |
| w.U8((byte)thing.FrameGroups.Length); | |
| for (int g = 0; g < thing.FrameGroups.Length; g++) | |
| int groupCount = thing.FrameGroups != null ? thing.FrameGroups.Length : 0; | |
| if (isOutfit && frameGroups) | |
| { | |
| if (groupCount == 0) | |
| throw new System.InvalidOperationException("Outfit thing with frameGroups enabled must have at least one FrameGroup."); | |
| w.U8((byte)groupCount); | |
| } | |
| int groupsToWrite = frameGroups ? groupCount : Math.Min(groupCount, 1); | |
| for (int g = 0; g < groupsToWrite; g++) |
| w.U32(i < fg.SpriteIndex.Length ? fg.SpriteIndex[i] : 0); | ||
| { | ||
| uint sid = i < fg.SpriteIndex.Length ? fg.SpriteIndex[i] : 0; | ||
| if (extended) w.U32(sid); else w.U16((ushort)sid); |
There was a problem hiding this comment.
When extended == false, sprite IDs are written as U16 by casting from uint. If any sprite ID exceeds ushort.MaxValue, this will silently truncate and corrupt the DAT. Add an explicit range check (and fail fast / clamp) when writing legacy (non-extended) DATs.
| if (extended) w.U32(sid); else w.U16((ushort)sid); | |
| if (extended) | |
| { | |
| w.U32(sid); | |
| } | |
| else | |
| { | |
| if (sid > ushort.MaxValue) | |
| { | |
| throw new System.InvalidOperationException( | |
| $"Sprite ID {sid} exceeds maximum value {ushort.MaxValue} for non-extended DAT."); | |
| } | |
| w.U16((ushort)sid); | |
| } |
| public void Seek(int position) => _pos = position; | ||
|
|
There was a problem hiding this comment.
DatReader.Seek sets _pos directly without any bounds/negative validation. A negative position can bypass EnsureAvailable checks (e.g., _pos + bytes > data.Length) and cause undefined reads. Add validation to keep position within [0, data.Length] (or throw) to keep the reader safe if Seek is used later.
Summary
Two features for the Assets & Map Editor:
1. Category-specific import menus
Split the single "Merge Session Into Current…" menu into 4 separate menus:
Each menu dynamically lists available session DAT files and imports only the selected category.
2. DAT parser: independent feature flags
The DAT parser previously derived
extended(U32 sprites),enhancedAnimations, andframeGroupsfrom the protocol version number. This failed for custom DAT files (e.g. PStory) that use hybrid combinations — V4 flags (proto 854) + U32 sprites + enhanced animations + frame groups — a mix no standard Tibia version uses.Root cause: PStory client treats these as independent feature flags and tries multiple combinations via
tryLoadDatWithFallbacks. Our parser now does the same.Before: Items=39100, Outfits=4970/5030, Effects=0/2208, Missiles=0/223
After: Items=39100, Outfits=5030/5030, Effects=2208/2208, Missiles=223/223
Changes
DatFile.cs:ParseThingaccepts separatebool extended,bool enhancedAnimations,bool frameGroups;Load()tries 8 feature combos per protocol;Save/WriteThingrespects flagsDatData: AddedEnhancedAnimationsandFrameGroupspropertiesMainWindow.axaml: 4 category-specific import MenuItemsMainWindow.axaml.cs:WireImportMenu()helperMainWindowViewModel.cs:MergeSessionAsyncacceptsThingCategory?filter