-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Move tutorial storage #14422
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?
Move tutorial storage #14422
Conversation
|
All right, "Tutorial "Religions" contains no text" - yes that's intentional mister unit test: Will think about later. |
| }, | ||
| { | ||
| "name": "Religions", | ||
| // The available Religions will be appended to any civilopediaText defined here by code |
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.
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.
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.
Yes I arrived at "simply feeding that page something" being the best solution for the unit test. I had thought emphasizing that neither symbol nor name have actual gameplay value. A quote might be nice, but I'd go with Pascal's "wager" one rather than copying anything, even if I think C4 or C3 did quote that...
More concept info should IMO stay in "concept" tutorials, and might be out of scope here. How all this reads in the end, and how it is accessible/findable, and whether a few more links or illustrations might help, I'll leave for someone else1.
Oooooohhhh - forgot to test TFW runs. TFW changes are exactly as expected.
Footnotes
-
Slartibartfast! ↩
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.
Cool, cleaned up the edges a bit over at SomeTroglodyte#11 .
| KeyboardBinding.PediaTutorials, | ||
| "OtherIcons/ExclamationMark", | ||
| { _, tutorialController, _ -> tutorialController.getCivilopediaTutorials() } | ||
| { ruleset, _ -> ruleset.getCivilopediaTutorials() } |
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.
Since the Religion one calls the local getCivilopediaReligionEntries(), I think it would be fine to make getCivilopediaTutorials() local as well. As in call it with the same signature as the other method: getCivilopediaTutorials(ruleset)
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.
But then to be really consistent there would be yet another one that's outsourced but could conceivably be local too, wasn't there?
|
I'm really not convinced that religion and espionage are not relevant for other rulesets, this actually seem to be general game mechanics and not particular to only G&K |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
f3ae750 to
46c1414
Compare
|
Conflicts have been resolved. |
46c1414 to
52690e9
Compare
|
Since the tutorial category thing made a merge unpleasant, plus that "general game mechanics and not particular to only G&K" argument, I've reduced1 this to the purely technical aspects. Vanilla- or G&K- specific tutorials would work, but are not used here. The screenies showing heads and quotes decorations are no longer relevant. They can come later - Rob has better PNG's anyway. Footnotes |



Related: #14420, #12834