add amplify icon and support for scheme_amplify #266
Conversation
|
I just commented on the issue that I think the original proposal for I have a guess that the other three properties used I know you've already merged changes to converge the JSON repo to use I think in this repo, we should make sure we add a property that is future-proof and makes sense and I think the name that best exemplifies that is |
| @@ -879,6 +882,7 @@ protected function importSideSchemeData(Card $card, $data) | |||
| 'escalation_threat_star', | |||
| 'scheme_acceleration', | |||
| 'scheme_crisis', | |||
| 'scheme_amplify', | |||
There was a problem hiding this comment.
I spent some effort making sure these keys are all alphabetical. Can you ensure they remain that way in this file? This looks to be out of order but if the name gets simplified to just amplify we should make sure all of these end up in the new correct order.
| @@ -78,6 +78,7 @@ public function serialize() { | |||
| $optionalFields[] = 'resource_wild'; | |||
| $optionalFields[] = 'scheme_acceleration'; | |||
| $optionalFields[] = 'scheme_crisis'; | |||
| $optionalFields[] = 'scheme_amplify'; | |||
There was a problem hiding this comment.
I believe we also want to add this to the ally case below because those cards (https://marvelcdb.com/card/44016) use this new property too
There was a problem hiding this comment.
I'm not sure adding "$optionalFields[] = 'scheme_amplify';" in Card.php is relevant
(although it is necessary in ImportStdCommand.php)
Lady Deadpool is well displayed in my local devenv as is:
| src: url('../fonts/marvel-icons.eot?4fzpq9'); | ||
| src: url('../fonts/marvel-icons.eot?4fzpq9#iefix') format('embedded-opentype'), | ||
| url('../fonts/marvel-icons.ttf?4fzpq9') format('truetype'), | ||
| url('../fonts/marvel-icons.woff?4fzpq9') format('woff'), | ||
| url('../fonts/marvel-icons.svg?4fzpq9#marvel-icons') format('svg'); |
There was a problem hiding this comment.
I don't know enough about these type of font files but if we delete these files here, they are orphaned in the repo and should probably get deleted.
I am curious though why we needed them in the first place (or if we still need them and shouldn't delete them) since this new ttf format matches the existing marvel-icons.ttf format and we still used eot, woof, and svg files before. Again, not a font file expert so I'm unfamiliar with the nuances of those types of files.
| url('../fonts/marvel-icons.woff?4fzpq9') format('woff'), | ||
| url('../fonts/marvel-icons.svg?4fzpq9#marvel-icons') format('svg'); | ||
|
|
||
| src: url('../fonts/ChampionsIcons.ttf?') format('truetype'); |
There was a problem hiding this comment.
I would think if we have a new icon that needs to get added we would replace the existing marvel-icons.ttf with this new file instead of creating a new file with a new name.
There was a problem hiding this comment.
I kept the exact name of the file from the homebrew server in case it is updated
Also changing the name of the file might help to force the upload if the previous file is in some cache
this should fix #250
and should fix #196
the source of the new font is:
from the Marvel Champions LCG Homebrew discord server
in the drive that contains templates
for consistency, card_schema.json should be updated in marvelsdb-json-data repo (zzorba/marvelsdb-json-data#574)
this has been tested in local env, as you can see on the following screen shots:
I am not a font specialist, so do not hesitate to propose improvments :)