diff --git a/Projects/UOContent.Tests/Tests/Multis/SpreadsheetTests.cs b/Projects/UOContent.Tests/Tests/Multis/SpreadsheetTests.cs new file mode 100644 index 000000000..3be9d9a0d --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Multis/SpreadsheetTests.cs @@ -0,0 +1,114 @@ +using System; +using System.IO; +using Server.Multis; +using Xunit; + +namespace UOContent.Tests; + +public class SpreadsheetTests : IDisposable +{ + // misc.txt's shape: 13 columns, the last being the Comment the client never reads. + private const string Types = "int\tint\tint\tint\tint\tint\tint\tint\tint\tint\tint\tint\tstring"; + + private const string Names = + "Category\tStyle\tTID\tPiece1\tPiece2\tPiece3\tPiece4\tPiece5\tPiece6\tPiece7\tPiece8\tFeatureMask\tComment"; + + private readonly string _path = Path.Combine(Path.GetTempPath(), $"muo-sheet-{Guid.NewGuid():N}.txt"); + + public void Dispose() + { + if (File.Exists(_path)) + { + File.Delete(_path); + } + } + + private Spreadsheet Write(params string[] rows) + { + var lines = new string[rows.Length + 2]; + lines[0] = Types; + lines[1] = Names; + rows.CopyTo(lines, 2); + + File.WriteAllLines(_path, lines); + return new Spreadsheet(_path); + } + + [Fact] + public void RowMissingTrailingCommentIsStillRead() + { + // An empty Comment written without a trailing tab leaves the row one field short. + var ss = Write("0\t0\t1060056\t44\t0\t41\t40\t42\t0\t43\t29\t8"); + + var record = Assert.Single(ss.Records); + + Assert.Equal(44, record.GetInt32(ss.GetColumnID("Piece1"))); + Assert.Equal(29, record.GetInt32(ss.GetColumnID("Piece8"))); + Assert.Equal(8, record.GetInt32(ss.GetColumnID("FeatureMask"))); + } + + [Fact] + public void ShortRowDoesNotDropLaterRows() + { + var ss = Write( + "0\t0\t1060056\t44\t0\t41\t40\t42\t0\t43\t29\t0", + "0\t1\t1060057\t45\t0\t0\t0\t0\t0\t0\t0\t0\tFieldstone Arches" + ); + + Assert.Equal(2, ss.Records.Length); + Assert.Equal(44, ss.Records[0].GetInt32(ss.GetColumnID("Piece1"))); + Assert.Equal(45, ss.Records[1].GetInt32(ss.GetColumnID("Piece1"))); + } + + [Fact] + public void TabOnlySeparatorLinesAreNotMistakenForHeaderRows() + { + // The retail client's doors.txt separates its header rows this way. + var separator = new string('\t', 12); + + File.WriteAllLines( + _path, + [ + Types, + separator, + Names, + separator, + "0\t0\t1060056\t44\t0\t41\t40\t42\t0\t43\t29\t0\tFieldstone Archways" + ] + ); + + var ss = new Spreadsheet(_path); + + Assert.Equal(3, ss.GetColumnID("Piece1")); + Assert.Equal(11, ss.GetColumnID("FeatureMask")); + + var record = Assert.Single(ss.Records); + Assert.Equal(44, record.GetInt32(ss.GetColumnID("Piece1"))); + } + + [Fact] + public void MissingHeaderRowsThrowsInsteadOfNullReference() + { + File.WriteAllLines(_path, [Types]); + + Assert.Throws(() => new Spreadsheet(_path)); + } + + [Fact] + public void HeaderWithFewerNamesThanTypesIsTolerated() + { + File.WriteAllLines( + _path, + [ + Types, + "Category\tStyle\tTID\tPiece1", + "0\t0\t1060056\t44\t0\t41\t40\t42\t0\t43\t29\t0\tFieldstone Archways" + ] + ); + + var ss = new Spreadsheet(_path); + + Assert.Equal(44, ss.Records[0].GetInt32(ss.GetColumnID("Piece1"))); + Assert.Equal(-1, ss.GetColumnID("FeatureMask")); + } +} diff --git a/Projects/UOContent/Multis/ComponentVerification.cs b/Projects/UOContent/Multis/ComponentVerification.cs index 967fed1bc..dda094aa4 100644 --- a/Projects/UOContent/Multis/ComponentVerification.cs +++ b/Projects/UOContent/Multis/ComponentVerification.cs @@ -4,11 +4,14 @@ using System.Collections.Generic; using System.IO; using System.IO.Compression; using Server.Compression; +using Server.Logging; namespace Server.Multis; public static class ComponentVerification { + private static readonly ILogger logger = LogFactory.GetLogger(typeof(ComponentVerification)); + private static int[] _itemTable; private static int[] _multiTable; private static bool _loaded; @@ -20,6 +23,15 @@ public static class ComponentVerification // encodes them) while AOS/SE/ML/... line up unchanged. private const int HousingTierMask = (int)HousingFlags.HousingEJ; + // Table sentinels. Slots start as NotAComponent, which CheckValidity rejects, so a piece that + // never gets registered can never be placed. NoFeatureRequired is a piece with no expansion + // requirement: walls.txt encodes pre-AOS base pieces as 0, housing.bin collapses to 0 under + // HousingTierMask. + private const int NotAComponent = -1; + private const int NoFeatureRequired = 0; + + private const string FeatureMaskColumn = "FeatureMask"; + public static bool IsItemValid(int itemID) { EnsureLoaded(); @@ -33,7 +45,8 @@ public static class ComponentVerification } private static bool CheckValidity(int val) => - val != -1 && (val == 0 || ((int)ExpansionInfo.CoreExpansion.HousingFlags & val) != 0); + val != NotAComponent && + (val == NoFeatureRequired || ((int)ExpansionInfo.CoreExpansion.HousingFlags & val) != 0); private static void EnsureLoaded() { @@ -42,22 +55,50 @@ public static class ComponentVerification return; } + // Set before loading: this runs from the design packet handler, so a bad file must not + // re-read every sheet on each later placement attempt. Sheets below fail independently. _loaded = true; _itemTable = CreateTable(TileData.MaxItemValue); _multiTable = CreateTable(0x4000); var housingPath = MultiData.HousingUOPPath; - if (housingPath != null) + if (housingPath != null && TryLoadFromHousingBin(housingPath)) { - var entry = MultiData.HousingEntry; - LoadFromHousingBin(ReadUOPEntry(housingPath, entry)); return; } LoadFromTxtFiles(); } + private static bool TryLoadFromHousingBin(string path) + { + try + { + var data = ReadUOPEntry(path, MultiData.HousingEntry); + if (data != null) + { + LoadFromHousingBin(data); + return true; + } + + logger.Warning( + "Could not decompress housing.bin from {Path}. Falling back to the component sheets", + path + ); + } + catch (Exception ex) + { + logger.Warning( + ex, + "Failed to read housing.bin from {Path}. Falling back to the component sheets", + path + ); + } + + return false; + } + private static byte[] ReadUOPEntry(string path, UOPEntry entry) { using var stream = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read); @@ -223,16 +264,55 @@ public static class ComponentVerification return; } - var ss = new Spreadsheet(path); + Spreadsheet ss; + try + { + ss = new Spreadsheet(path); + } + catch (Exception ex) + { + // One unreadable sheet must not take the others down with it. + logger.Error(ex, "Could not read house components from {Path}", path); + return; + } + + // GetInt32 on a missing column yields NoFeatureRequired, which would register every piece in + // the sheet as unconditionally placeable. Refuse the sheet instead. + var featureCID = ss.GetColumnID(FeatureMaskColumn); + if (featureCID < 0) + { + logger.Error( + "House component sheet {Path} has no {Column} column. Its pieces will not be registered", + path, + FeatureMaskColumn + ); + return; + } + + // An individual missing column is expected - older sheets predate walls.txt's + // SecondAltWindowS/E - but a sheet matching none of them is not the sheet we expect. var tileCIDs = new int[tileColumns.Length]; + var matchedColumns = 0; for (var i = 0; i < tileColumns.Length; ++i) { tileCIDs[i] = ss.GetColumnID(tileColumns[i]); + + if (tileCIDs[i] >= 0) + { + matchedColumns++; + } } - var featureCID = ss.GetColumnID("FeatureMask"); + if (matchedColumns == 0) + { + logger.Error( + "House component sheet {Path} has none of its expected tile columns. Its pieces will not be registered", + path + ); + return; + } for (var i = 0; i < ss.Records.Length; ++i) { @@ -260,7 +340,7 @@ public static class ComponentVerification for (var i = 0; i < table.Length; ++i) { - table[i] = -1; + table[i] = NotAComponent; } return table; @@ -277,11 +357,18 @@ public class Spreadsheet var types = ReadLine(ip); var names = ReadLine(ip); + if (types == null || names == null) + { + throw new InvalidDataException($"House component sheet '{path}' is missing its header rows."); + } + m_Columns = new ColumnInfo[types.Length]; for (var i = 0; i < m_Columns.Length; ++i) { - m_Columns[i] = new ColumnInfo(i, types[i], names[i]); + // A names row shorter than the types row leaves the extras unnamed, so nothing resolves + // to them. + m_Columns[i] = new ColumnInfo(i, types[i], i < names.Length ? names[i] : ""); } var records = new List(); @@ -294,10 +381,15 @@ public class Spreadsheet { var ci = m_Columns[i]; + // Client sheets write an empty trailing Comment as a plain newline, leaving the row + // one field short. The client only requires the columns up to FeatureMask, so treat + // the missing field as empty rather than dropping a row that lists real pieces. + var value = ci.m_DataIndex < values.Length ? values[ci.m_DataIndex] : null; + data[i] = ci.m_Type switch { - "int" => Utility.ToInt32(values[ci.m_DataIndex]), - "string" => values[ci.m_DataIndex], + "int" => Utility.ToInt32(value), + "string" => value, _ => data[i] }; } @@ -327,7 +419,10 @@ public class Spreadsheet { while (ip.ReadLine() is { } line) { - if (line.Length > 0) + // Whitespace-only, not merely empty: the retail client's doors.txt separates its header + // rows with lines of bare tabs, and accepting one as the names row leaves every column + // unnamed, so no door resolves. The client skips them the same way. + if (!string.IsNullOrWhiteSpace(line)) { return line.Split('\t'); }