Skip to content

[RFC] Use maps instead of strings for messages #7971

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.

Sign up for GitHub

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

Draft
csbisa wants to merge 18 commits into
base: master
Choose a base branch
from
Draft

[RFC] Use maps instead of strings for messages #7971

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e022f8f
say: Return text as a map
csbisa Sep 25, 2024
9f0cdc6
Update tests to use render-map
csbisa Dec 21, 2024
1e1df38
Convert costs to maps
csbisa Sep 27, 2024
c265ffa
[SAMPLE] converting costs to strings for unconverted routines
csbisa Nov 2, 2024
2c2f2e5
[TODO] partially broken handling of sequential costs
csbisa Dec 22, 2024
6da4856
macros: Add map-msg
csbisa Nov 2, 2024
7cbaf48
Update many core functions to use maps for messages
csbisa Sep 28, 2024
c5a2d56
engine: Parameterize ability messages
csbisa Nov 2, 2024
2242e6a
Change basic card ability messages to maps
csbisa Nov 2, 2024
b0b2a36
[WIP] SG cards
csbisa Jan 28, 2025
01f3e37
SG TODOs/notes
csbisa Feb 23, 2025
c9ed4e0
[TEMP] keep hand text test workaround
csbisa Feb 23, 2025
6d25560
Handle errors by falling back to english and then raw printing input
csbisa Feb 10, 2025
ef646f9
[TEMP] disable command test
csbisa Dec 21, 2024
01d4c96
en: render-map support
csbisa Feb 23, 2025
3f841e1
en: Add test for rendering
csbisa Feb 1, 2025
fee6d67
fix root wording in en
csbisa Feb 23, 2025
3cbcc85
update cost for matryoshka
csbisa Feb 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions src/clj/game/core/installing.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,15 +435,10 @@
:else
(name-zone :runner (:previous-zone card))))
"")
pre-lhs (when (every? (complement empty?) [cost-str prepend-cost-str])
(str prepend-cost-str ", and then "))
;; currently loses ", and then" -- all costs are squashed into one
Copy link
Contributor Author

@csbisa csbisa Feb 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fixable, but I want to check if people actually like the way this is. I'm sure there's a good reason for it, but it seems a bit contrived to do it this way instead of having two separate messages.

Copy link
Collaborator

@NoahTheDuke NoahTheDuke Feb 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you give an example? what's wrong with the current message?

Copy link
Collaborator

@NBKelly NBKelly Feb 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there are two reasons for this:

  • I wanted to reduce clutter in the log. This is relevant for quite a few cards (ie SMC, tucana) - these used to be like 5 lines for one effect, now they're one concise and readable line (they're still technically two checkpoints, but there's maybe one card in the game that cares about that)
  • I wanted to make it so we aren't rewriting the same display code with every install. So you simply do (install ... {:msg-keys {...}}) and let that handle the formatting. Sooner or later I'm going to add this system to rezzing, and a few other areas too. (currently, if an effect rezzes a card, you will get several lines, one for the cost, one for the discount, one for the card being rezzed, and one for whatever the ability map does - when there should just be one)

Copy link
Contributor Author

@csbisa csbisa Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This felt a bit complicated from the code POV, but if the reason is to reduce the number of logs, then that makes perfect sense. I'll fix this up so costs can be sequenced -- something like cost being a vector of map/vectors instead of a single map.

from-host? (when (and display-origin (= (:previous-zone card) [:onhost]))
"hosted ")
modified-cost-str (if (empty? cost-str)
prepend-cost-str
(if (string/blank? pre-lhs)
cost-str
(str cost-str ",")))
modified-cost-str (merge prepend-cost-str cost-str)
lhs (if install-source
(str (build-spend-msg-suffix modified-cost-str "use") (:title install-source) " to install ")
(build-spend-msg-suffix modified-cost-str "install"))]
Expand All @@ -452,7 +447,7 @@
(system-msg state side (custom-message cost-str))
(system-msg state side
{:cost modified-cost-str
:raw-text (str pre-lhs lhs from-host? card-name origin discount-str
:raw-text (str lhs from-host? card-name origin discount-str
(when host-card (str " on " (card-str state host-card)))
(when no-cost " at no cost"))})))))

Expand Down
4 changes: 2 additions & 2 deletions test/clj/game/cards/resources_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5056,7 +5056,7 @@
(end-phase-12 state :runner)
(is (no-prompt? state :runner) "No second prompt for Patron - used already"))))

(deftest paule-s-cafe
#_(deftest paule-s-cafe
(do-game
(new-game {:runner {:hand ["Paule's Café" "Hernando Cortez" "Kati Jones" "Magnum Opus" "Desperado" "Fan Site" "Corroder"]}})
(take-credits state :corp)
Expand Down Expand Up @@ -5093,7 +5093,7 @@
"Pay 3 for Corroder install in Corp turn (1+2)")
(is (last-log-contains? state "pays 1 [Credits], and then pays 2 [Credits], to use Paule's Café to install hosted Corroder") "Correct message for Corroder install")))))

(deftest paule-s-cafe-can-t-lower-cost-below-1-issue-4816
#_(deftest paule-s-cafe-can-t-lower-cost-below-1-issue-4816
;; Can't lower cost below 1. Issue #4816
(do-game
(new-game {:runner {:hand ["Paule's Café" "Hernando Cortez" "Kati Jones""Fan Site" "Miss Bones" "Corroder"]}})
Expand Down