-
Notifications
You must be signed in to change notification settings - Fork 251
Enhance snippet handling #7086
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: develop
Are you sure you want to change the base?
Enhance snippet handling #7086
Changes from all commits
41478e0
90cb66e
bb0d562
838baa2
4c0d80f
9730881
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Your explanation here (#XYZW). |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,13 @@ | ||||||||||||||
| #module Jekyll | ||||||||||||||
| # class BalanceChangeBlock < Jekyll::Hooks | ||||||||||||||
| # end | ||||||||||||||
| #end | ||||||||||||||
|
|
||||||||||||||
| Jekyll::Hooks.register [:pages, :documents], :post_render do |doc| | ||||||||||||||
|
|
||||||||||||||
| doc.output.gsub!( | ||||||||||||||
| /([^<\s]+)\s*-->\s*([^<\s]+)/, | ||||||||||||||
| '<span class="old">\1</span> → <span class="new">\2</span>' | ||||||||||||||
|
Comment on lines
+8
to
+10
Contributor
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. Regex misses common value formats with spaces/parentheses. This pattern only matches single tokens, so examples like 🔧 Suggested patch- /([^<\s]+)\s*-->\s*([^<\s]+)/,
+ /([^<]+?)\s*-->\s*([^<]+?)(?=\s*(?:<|$))/,📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| end | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| module Jekyll | ||
| class UnitBlock < Liquid::Block | ||
|
|
||
| def initialize(tag_name, markup, tokens) | ||
| super | ||
| @unit_id = markup.strip | ||
| end | ||
|
|
||
| def render(context) | ||
| name = super.strip | ||
|
|
||
| <<~HTML | ||
| <div class="unit-header" data-unit="#{@unit_id}"> | ||
| <img class="unit-icon" | ||
| src="/assets/icons/#{@unit_id}.png"> | ||
| <span class="unit-name">#{name}</span> | ||
|
Comment on lines
+13
to
+16
Contributor
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. Escape and validate interpolated HTML values.
🛡️ Suggested patch+require "cgi"
+
module Jekyll
class UnitBlock < Liquid::Block
@@
def render(context)
- name = super.strip
+ name = CGI.escapeHTML(super.strip)
+ unit_id = `@unit_id.strip`
+ raise ArgumentError, "Invalid unit id" unless unit_id.match?(/\A[a-zA-Z0-9_\/-]+\z/)
+ safe_unit_id = CGI.escapeHTML(unit_id)
<<~HTML
- <div class="unit-header" data-unit="#{`@unit_id`}">
+ <div class="unit-header" data-unit="#{safe_unit_id}">
<img class="unit-icon"
- src="/assets/icons/#{`@unit_id`}.png">
+ src="/assets/icons/#{safe_unit_id}.png"
+ alt="">
<span class="unit-name">#{name}</span>
</div>
HTML🤖 Prompt for AI Agents |
||
| </div> | ||
| HTML | ||
| end | ||
| end | ||
| end | ||
|
|
||
| Liquid::Template.register_tag('unit', Jekyll::UnitBlock) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| .unit-header { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 10px; | ||
| margin-top: 2rem; | ||
| font-size: 1.2rem; | ||
| font-weight: 600; | ||
| } | ||
|
|
||
| .unit-icon { | ||
| width: 64px; | ||
| height: 64px; | ||
| } | ||
|
|
||
| .unit-name { | ||
| display: inline-block; | ||
| } | ||
|
|
||
| .unit-header + ul > li { | ||
| font-weight: 600; | ||
| } | ||
|
|
||
| .old { | ||
| color: #d73a49; | ||
| text-decoration: line-through; | ||
| } | ||
|
|
||
| .new { | ||
| color: #2da44e; | ||
| font-weight: 600; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,23 +6,57 @@ permalink: development/changelog/balance-snippet | |||||||||||
| published: true | ||||||||||||
| --- | ||||||||||||
|
|
||||||||||||
| _Information about how to structure a balance snippet_ | ||||||||||||
| The game just renders the text as is, so we need to keep that in mind. | ||||||||||||
| Unit or explanation first? | ||||||||||||
| The unitID is also used to infer if the snippet is supposed to go into the land, navy, air, structures or enhancements category. | ||||||||||||
| if the change is an enhancement, the unitID is enhancements/faction/name | ||||||||||||
| The md to lua converter should remove the {: .unit-change} and the (unitID) | ||||||||||||
|
|
||||||||||||
|
Comment on lines
+9
to
+14
Contributor
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. Remove draft notes before publishing this doc page. These lines read like internal brainstorming/TODO notes and conflict with the otherwise polished guidance. 🧾 Suggested cleanup-The game just renders the text as is, so we need to keep that in mind.
-Unit or explanation first?
-The unitID is also used to infer if the snippet is supposed to go into the land, navy, air, structures or enhancements category.
-if the change is an enhancement, the unitID is enhancements/faction/name
-The md to lua converter should remove the {: .unit-change} and the (unitID)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| The snippet file should be named `balance.<PR Number>.md`. | ||||||||||||
|
|
||||||||||||
| ``` | ||||||||||||
|
Contributor
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. Fix markdownlint issues in examples section. Line 19 should specify a fenced language, and Line 38 should not jump heading levels. 🧹 Suggested patch-```
+```md
@@
-### Example snippets
+## Example snippetsAlso applies to: 38-38 🧰 Tools🪛 markdownlint-cli2 (0.22.0)[warning] 19-19: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🤖 Prompt for AI Agents |
||||||||||||
| - (#<PR Number>) <Description> | ||||||||||||
|
|
||||||||||||
| **<Formatted Unit Name> (<Blueprint ID>):** | ||||||||||||
| - <Section> | ||||||||||||
| - <Parameter Name>: <value before> --> <value after> | ||||||||||||
| - <Parameter Name>: <value before> --> <value after> | ||||||||||||
| {% unit <BlueprintID> %} | ||||||||||||
| <Formatted Unit Name> | ||||||||||||
| {% endunit %} | ||||||||||||
| <Description> (#<PR Number>) | ||||||||||||
| - <Category>: | ||||||||||||
| - <Parameter Name>: <value before> --> <value after> | ||||||||||||
| ``` | ||||||||||||
|
|
||||||||||||
| - PR Number: The number of the pull request on GitHub. | ||||||||||||
| - Description: A 1-sentence summary of the changes and then a short explanation behind the reasoning for the changes. | ||||||||||||
| - Formatted Unit Name: `<UnitName>: <Tech> <Unit Description>` For example: `Exodus: T2 Destroyer`. This is similar to the format visible in unit dbs and the in-game UI when hovering over a unit. | ||||||||||||
| - Blueprint ID: Blueprint ID of the unit in uppercase. | ||||||||||||
| - Section: An optional subheader to categorize parameters. Usually a blueprint subtable name (ex: Physics) or a weapon name. | ||||||||||||
| - Parameter Name: The name for the value that was changed. It shouldn't be the exact blueprint field name; it should be a name that players can understand and formatted like normal text. | ||||||||||||
| - Formatted Unit Name: `<UnitName>: <Tech> <Unit Role>` For example: `Exodus: T2 Destroyer`. This is similar to the format visible in unit dbs and the in-game UI when hovering over a unit. | ||||||||||||
| - Blueprint ID: Blueprint ID of the unit. | ||||||||||||
| - Category: A subheader to categorize parameters. Usually a blueprint subtable name (ex: Physics) or a weapon name. | ||||||||||||
| - Parameter Name: The name for the value that was changed. It doesn't need to be the exact blueprint field name; it should be a name that players can understand. | ||||||||||||
| - Value before/after: The value before/after the change. If relevant, derived values like DPS can be put in parentheses after the value as such: `<damage> (<dps>) --> <damage> (<dps>)`. | ||||||||||||
|
|
||||||||||||
| When the same change has been applied to multiple units, you can just write the group in the title. The unitID still loads the unit icon, so choose a unit that is most representative. (UEF when all factions are affected). | ||||||||||||
|
|
||||||||||||
| ### Example snippets | ||||||||||||
|
|
||||||||||||
| {% unit URS0201 %} | ||||||||||||
| Salem Class: T2 Destroyer | ||||||||||||
| {% endunit %} | ||||||||||||
| Reduced Salem’s anti-torpedo flare target check interval from 1.0s to 0.4s—the standard for anti-projectile weapons. This improves torpedo detection and flare response, especially against torpedo bombers. In turn the movement speed has been tuned down (#6339). | ||||||||||||
| - Anti Torpedo: | ||||||||||||
| - Target Check Interval: 1s --> 0.4s | ||||||||||||
| - Movement: | ||||||||||||
| - Max speed : 5 --> 4 | ||||||||||||
|
|
||||||||||||
| {% unit UEA0102 %} | ||||||||||||
| All T1 Interceptors | ||||||||||||
| {% endunit %} | ||||||||||||
| Reduce the distance at which T1 Interceptors hover instead of turning when given a move order (#6342). | ||||||||||||
| - Air Movement: | ||||||||||||
| - Start Turn Distance: 10 --> 5 | ||||||||||||
|
|
||||||||||||
| {% unit enhancements/cybran/torp %} | ||||||||||||
| Nanite Torpedo Launcher | ||||||||||||
| {% endunit %} | ||||||||||||
| Further increase the MuzzleSalvoSize of the Cybran ACU's Nanite Torpedo upgrade to 4, as it still had difficulties penetrating torpedo defenses after (#6476) increased it to 3. Its DPS remains unchanged (#6542). | ||||||||||||
| - Torpedo weapon: | ||||||||||||
| - Damage (DPS): 60 (225) --> 45 (225) | ||||||||||||
| - Muzzle Salvo Size: 3 --> 4 | ||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,4 +6,13 @@ permalink: development/changelog/other-snippet | |||||||
| published: true | ||||||||
| --- | ||||||||
|
|
||||||||
| _Information about how to structure all other snippets_ | ||||||||
| The content of a snippet that is not in the balance category is similar to a commit message. The first line is a title that starts with a dash followed by a concise description of the changes. Use a dot at the end of the first line. The remainder of the file can be used to provide additional and more detailed information about the changes. Remember to indent these additional lines, so they follow the indentation that gets created because of the list item of the first line. Mention the relevant pull requests at the end of the text like so: `(#PR1, #PR2, ...)`. | ||||||||
| Add an empty line at the end of the file to make sure that the next snippet is separated by an empty line. You can make use of a Markdown formatter to ensure consistency, one example is the use of [prettier](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode). | ||||||||
|
|
||||||||
| ### Example snippet | ||||||||
| ``` | ||||||||
|
Comment on lines
+12
to
+13
Contributor
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. Fix markdownlint violations in the example section. Line 12 should not skip heading levels, and Line 13 should declare a fence language. 🧹 Suggested patch-### Example snippet
-```
+## Example snippet
+```md
- Tweak the formula used to calculate the level of detail (LOD) for props.📝 Committable suggestion
Suggested change
🧰 Tools🪛 markdownlint-cli2 (0.22.0)[warning] 12-12: Heading levels should only increment by one level at a time (MD001, heading-increment) [warning] 13-13: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🤖 Prompt for AI Agents |
||||||||
| - Tweak the formula used to calculate the level of detail (LOD) for props. | ||||||||
|
|
||||||||
| Large props are visible for longer, and it should now be easier again to spot broken tree groups (#6906). | ||||||||
|
|
||||||||
| ``` | ||||||||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: FAForever/fa
Length of output: 693
🏁 Script executed:
cat -n .github/workflows/auto-draft-pr.yaml | head -50Repository: FAForever/fa
Length of output: 2288
Workflow will fail: missing checkout, no push, and invalid filename pattern.
The job is missing a checkout step (line 34 tries to access
fa/changelog/snippetswithout cloning the repo first), never pushes the commit (line 46), and usescategory.<PR>.mdwhich the bundler doesn't recognize—it only processesbalance.*.md,features.*.md,fix.*.md,graphics.*.md,ai.*.md,performance.*.md, orother.*.md.Suggested fix
🤖 Prompt for AI Agents