-
-
Notifications
You must be signed in to change notification settings - Fork 78
Skills with a CELL pattern hit their pattern, not a single target #2289
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: master
Are you sure you want to change the base?
Changes from 4 commits
9b4a4da
56f02fb
8a6092b
3799d1e
2f5177d
c766113
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| using Microsoft.EntityFrameworkCore.Migrations; | ||
|
|
||
| #nullable disable | ||
|
|
||
| namespace NosCore.Database.Migrations | ||
| { | ||
| /// <inheritdoc /> | ||
| public partial class AddSkillCellPattern : Migration | ||
| { | ||
| /// <inheritdoc /> | ||
| protected override void Up(MigrationBuilder migrationBuilder) | ||
| { | ||
| migrationBuilder.AddColumn<string>( | ||
| name: "CellPattern", | ||
| table: "Skill", | ||
| type: "character varying(512)", | ||
| maxLength: 512, | ||
| nullable: true); | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| protected override void Down(MigrationBuilder migrationBuilder) | ||
| { | ||
| migrationBuilder.DropColumn( | ||
| name: "CellPattern", | ||
| table: "Skill"); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| // __ _ __ __ ___ __ ___ ___ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Remove UTF-8 BOMs from the new C# files.
As per coding guidelines, “No UTF-8 BOM on 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| // | \| |/__\ /' _/ / _//__\| _ \ __| | ||
| // | | ' | \/ |`._`.| \_| \/ | v / _| | ||
| // |_|\__|\__/ |___/ \__/\__/|_|_\___| | ||
| // | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Globalization; | ||
|
|
||
| namespace NosCore.GameObject.Services.BattleService; | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment is having some data that is just example not sure how useful those are
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All gone in 2f5177d — the four you named plus the file header and the two in the tests, so the whole PR is comment-free now. The two things that were genuinely non-obvious moved into the PR description instead: the CELL triples end on a Build clean, 1004 tests green. |
||
| // A skill's cell pattern, rotated towards the target. | ||
| // | ||
| // 67 of the 68 skills with a CELL section declare AOE with radius zero, because the area IS | ||
| // the pattern. Reading the radius makes them hit a single target and nothing says so. | ||
| // | ||
| // Rotation is by the angle to the target, exact on the cardinals and rounded on the | ||
| // diagonals, where a one-cell row can come out as a staircase with a gap at its side. | ||
| public static class SkillCells | ||
| { | ||
| // A malformed value yields null rather than throwing: the pattern decides who a skill hits. | ||
| public static sbyte[]? Parse(string? pattern) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(pattern)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var parts = pattern.Split(','); | ||
|
|
||
| // Pairs, so an odd count is a broken row, not a pattern with a spare coordinate. | ||
| if (parts.Length == 0 || parts.Length % 2 != 0) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var cells = new sbyte[parts.Length]; | ||
| for (var i = 0; i < parts.Length; i++) | ||
| { | ||
| if (!sbyte.TryParse(parts[i], NumberStyles.Integer, CultureInfo.InvariantCulture, | ||
| out cells[i])) | ||
| { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| return cells; | ||
| } | ||
|
|
||
| // The absolute cells hit. Caster and target on one cell leaves no direction: kept facing north. | ||
| public static HashSet<(short X, short Y)> Resolve(sbyte[] pattern, short casterX, | ||
| short casterY, short targetX, short targetY) | ||
| { | ||
| var cells = new HashSet<(short, short)>(pattern.Length / 2); | ||
|
|
||
| double dx = targetX - casterX; | ||
| double dy = targetY - casterY; | ||
| var len = Math.Sqrt((dx * dx) + (dy * dy)); | ||
|
|
||
| // At zero distance there is no direction: north, as authored. | ||
| double ux = 0, uy = -1; | ||
| if (len > 0.0001) | ||
| { | ||
| ux = dx / len; | ||
| uy = dy / len; | ||
| } | ||
|
|
||
| // Authored basis is right = (1,0), forward = (0,-1); forward maps onto (ux, uy) and right | ||
| // onto (-uy, ux). | ||
| foreach (var (cx, cy) in Pairs(pattern)) | ||
| { | ||
| double right = cx; | ||
| double forward = -cy; | ||
|
|
||
| var x = (right * -uy) + (forward * ux); | ||
| var y = (right * ux) + (forward * uy); | ||
|
|
||
| cells.Add(((short)(casterX + Math.Round(x, MidpointRounding.AwayFromZero)), | ||
| (short)(casterY + Math.Round(y, MidpointRounding.AwayFromZero)))); | ||
| } | ||
|
|
||
| return cells; | ||
| } | ||
|
|
||
| private static IEnumerable<(sbyte X, sbyte Y)> Pairs(sbyte[] pattern) | ||
| { | ||
| for (var i = 0; i + 1 < pattern.Length; i += 2) | ||
| { | ||
| yield return (pattern[i], pattern[i + 1]); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,13 @@ public IReadOnlyList<IAliveEntity> Resolve(IAliveEntity attacker, IAliveEntity p | |
| return results; | ||
| } | ||
|
|
||
| // A CELL pattern comes with radius zero: the area is the pattern. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. useless comment |
||
| var pattern = SkillCells.Parse(skill.CellPattern); | ||
| var cells = pattern == null | ||
| ? null | ||
| : SkillCells.Resolve(pattern, attacker.PositionX, attacker.PositionY, | ||
| primaryTarget.PositionX, primaryTarget.PositionY); | ||
|
|
||
| var range = skill.TargetRange; | ||
| var cx = primaryTarget.PositionX; | ||
| var cy = primaryTarget.PositionY; | ||
|
|
@@ -44,7 +51,7 @@ public IReadOnlyList<IAliveEntity> Resolve(IAliveEntity attacker, IAliveEntity p | |
| if (monster.VisualId == primaryTarget.VisualId && monster.VisualType == primaryTarget.VisualType) continue; | ||
| if (!monster.IsAlive) continue; | ||
| if (!IsEnemy(attacker, monster)) continue; | ||
| if (WithinRange(cx, cy, monster.PositionX, monster.PositionY, range)) | ||
| if (IsHit(cells, cx, cy, monster.PositionX, monster.PositionY, range)) | ||
| { | ||
| results.Add(monster); | ||
| } | ||
|
|
@@ -61,7 +68,7 @@ public IReadOnlyList<IAliveEntity> Resolve(IAliveEntity attacker, IAliveEntity p | |
| if (player.VisualId == attacker.VisualId && attacker.VisualType == VisualType.Player) continue; | ||
| if (!player.IsAlive) continue; | ||
| if (!IsEnemy(attacker, player)) continue; | ||
| if (WithinRange(cx, cy, player.PositionX, player.PositionY, range)) | ||
| if (IsHit(cells, cx, cy, player.PositionX, player.PositionY, range)) | ||
| { | ||
| results.Add(player); | ||
| } | ||
|
|
@@ -86,6 +93,13 @@ private static bool IsEnemy(IAliveEntity attacker, IAliveEntity candidate) | |
| }; | ||
| } | ||
|
|
||
| // The pattern's cells when the skill has one, otherwise the box around the target. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. useless comment |
||
| private static bool IsHit(HashSet<(short X, short Y)>? cells, short cx, short cy, short x, | ||
| short y, int range) | ||
| { | ||
| return cells != null ? cells.Contains((x, y)) : WithinRange(cx, cy, x, y, range); | ||
| } | ||
|
|
||
| private static bool WithinRange(short cx, short cy, short x, short y, int range) | ||
| { | ||
| return Math.Abs(cx - x) <= range && Math.Abs(cy - y) <= range; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,7 @@ public FluentParserBuilder<SkillDto> BuildParser(string folder) | |
| .Field(x => x.Type, chunk => Convert.ToByte(chunk["TYPE"][0][5])) | ||
| .Field(x => x.Element, chunk => Convert.ToByte(chunk["TYPE"][0][7])) | ||
| .Field(x => x.Combo, chunk => AddCombos(chunk)) | ||
| .Field(x => x.CellPattern, chunk => ReadCellPattern(chunk)) | ||
| .Field(x => x.CpCost, chunk => chunk["COST"][0][2] == "-1" ? (byte)0 : byte.Parse(chunk["COST"][0][2])) | ||
| .Field(x => x.Price, chunk => Convert.ToInt32(chunk["COST"][0][3])) | ||
| .Field(x => x.CastEffect, chunk => Convert.ToInt16(chunk["EFFECT"][0][3])) | ||
|
|
@@ -144,6 +145,48 @@ private List<BCardDto> AddBCards(Dictionary<string, string[][]> chunks) | |
| return list; | ||
| } | ||
|
|
||
| // CELL holds thirty cells at most; a longer pattern continues in the unused tail of | ||
| // COST. A continues flag on the last available triple means the row ran out, not that | ||
| // there is more, so the tail is read and may well be empty. | ||
| private static string? ReadCellPattern(Dictionary<string, string[][]> chunks) | ||
| { | ||
| if (!chunks.TryGetValue("CELL", out var cell) || cell.Length == 0) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var cells = new List<int>(); | ||
| var ranOut = ReadTriples(cell[0], 4, cells); | ||
|
|
||
| if (ranOut && chunks.TryGetValue("COST", out var cost) && cost.Length > 0) | ||
| { | ||
| ReadTriples(cost[0], 5, cells); | ||
| } | ||
|
|
||
| return cells.Count == 0 ? null : string.Join(",", cells); | ||
| } | ||
|
|
||
| // Field [0] is empty and [1] is the section name, so CELL's triples start at [4] and | ||
| // COST's tail at [5]. | ||
| private static bool ReadTriples(string[] fields, int start, List<int> into) | ||
| { | ||
| for (var i = start; i + 2 < fields.Length; i += 3) | ||
| { | ||
| if (!int.TryParse(fields[i], out var dx) | ||
| || !int.TryParse(fields[i + 1], out var dy) | ||
| || !int.TryParse(fields[i + 2], out var continues) | ||
| || continues == 0) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| into.Add(dx); | ||
| into.Add(dy); | ||
| } | ||
|
Comment on lines
+175
to
+185
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Discard the complete pattern after an invalid triple. If a later Make the parse result distinguish an invalid field from a zero terminator. Return 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
|
|
||
| return true; | ||
| } | ||
|
|
||
| // FCOMBO's first field is a switch (has a chain / has not), not a step. The row starts | ||
| // at index 2, so triplet j starts at 3 + j*3; counting from the switch shifted every | ||
| // step by one field and the chain never fired. | ||
|
|
||
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.
useless comment