'usa' image files are displayed very small.

[pixelzoom]

While working on renaming localized image files for phetsims/joist#953, I discovered that the 'usa' images for those sims look very wrong, very tiny. I stashed my work, and verified that I had not introduced the problem. I ended up reverting my work, and not commiting.

For example, see the first screenshot below for CAV and 'usa'. The "kicker" is very tiny.

The 'africa' and 'africaModest' images look fine. See the second screenshot below. I'm wondering why those files are PNG, while the 'usa' files are SVG, and whether that has anything to do with this problem.

In any case, this needs to be fixed before we proceed with moving to the new approach for localized images. Assigning to the responsible devs for soccer-common with high priority.

[pixelzoom]

I see several commits to soccer-common on Wed 3/6/24 by @amanda-phet and @Luisav1, all related to these image files. If I checkout soccer-common prior to those commits (git checkout 86c60a3) then the problem goes away. So assigning this issue to @amanda-phet and @Luisav1.

[pixelzoom]

Rather than toss all of my work... In b9dee33, I went ahead and renamed the 'africa' and 'africaModest' image files, since they were not involved in the 3/6 commits.

@amanda-phet @Luisav1 please let me know when the 'usa' images are fixed, and I will renamed them by adding the 'usa' prefix.

[marlitas]

This is currently being caused because some images were migrated to svg and some are still png. @amanda-phet will be adding in the rest of the svg kicker images soon, and I imagine she can follow the new naming conventions while doing so.

[pixelzoom]

Thanks @marlitas. I went ahead and rename the 'usa' image files in e4a8d60.

[pixelzoom]

Btw... It's a little confusing that you refer to these images as "kickers", but all of the files names are "player". And the asserts files are kicker-artwork.ai and kicker-artwork-africa.ai.

[marlitas]

Mmm that is confusing... @amanda-phet do we want to update the names of the images to kickers? That's how they are referenced throughout the code... Feels like an easier change than refactoring all of the code to players...

[pixelzoom]

If you're going to rename files to "kicker", I recommend doing so with a bash script (see below), and using git mv so that git history is preserved. license.json and Kicker*.ts files can quickly be updated using strings search/replace in WebStorm.

#!/bin/bash

# Allow patterns that match no files, so we can iterate over both .png and .svg
shopt -s nullglob

for filename in *.{png,svg}; do

  # Change 'Player' to 'Kicker'.
 NEW_FILENAME=`echo ${filename} | sed "s/Player/Kicker/g"`

 # Rename the file, preserving git history.
 git mv ${filename} ${NEW_FILENAME}
done

[marlitas]

I'm confused... the only files I see named player are image assets... you want me to preserve the git history for those? Happy to, just don't fully understand the need to do so.

[pixelzoom]

Git history is not just for code. Why would you not preserve the git history for images? If you wanted to verify the info in license.json, or revert to a previous verison of an image, for example.

And speaking of license.json, I've noticed that most of the dates in license.json appear to be incorrect for soccer-common. For example, every entry in images/africa/license.json is Copyright 2022, but none of the image in images/africa/ existed until 8/30/2023

[marlitas]

Reverting to a previous version of an image, doesn't seem like something that I would want to do, since I would hope it matches the assets file as much as possible, however I see your reasoning and will make sure that the git history is preserved in the renames.

[pixelzoom]

In ee8ac8f, image files were rename from "Player" to "Kicker".

Slack:

@pixelzoom

Do you want me to change “player” => “kicker” in soccer-common?

@amanda-phet

I can go either way with player/kicker

@pixelzoom

The asset files are kicker, the code is kicker…. and the images files are player.

@amanda-phet

Right, I saw the issue, and I’m fine with renaming those now or later it doesn’t matter to me

[pixelzoom]

In order to proceed with conversion to the new Region and Culture approach, so I can address phetsims/center-and-variability#615 (migration rules)...

In the above commits, I integrated new SVG files for africa and africaModest, provided by @amanda-phet. The screenshots below were provided by @amanda-phet, and show how some of the kicker numbers map to images for other regions.

After making these changes, the original problem was not fixed. And now the kickers for all regions are tiny.

africa:

africaModest:

[pixelzoom]

After making these changes, the original problem was not fixed. And now the kickers for all regions are tiny.

It looks like this constant in KickerNode.ts was the culprit:

const SCALE = 0.31;

Using the published version (1.1.5) as a reference, I changed this value to 0.645.

@marlitas please review. Close if OK.

[pixelzoom]

In KickerNode.ts, I also see this comment. Should this be addressed?

      // Avoid a flickering on firefox where the image temporarily disappears (even in built mode)
      // TODO: once svg images are in, remove webgl flag and send to QA, https://github.com/phetsims/soccer-common/issues/11
      renderer: 'webgl',

[amanda-phet]

The kickers still looks small to me on main. We knew why they were small, but @Luisav1 did what you did and when the images were full sized they looked blurry (which shouldn't happen with an SVG!). We decided that after all the SVGs were in we would remove webgl and see if they looked better and no longer had an issue in firefox.

[pixelzoom]

The kickers still looks small to me on main.

Here's production, https://phet.colorado.edu/sims/html/center-and-variability/latest/center-and-variability_all.html?debugger:

Here's main:

[marlitas]

Yes the bug was a combination of consistent image sizes and then updating the constant once that was done and we knew what the new dimensions would be. I should have been more specific. Thanks @pixelzoom.

Kicker sizes look good on main to me! Thanks.

I'll go ahead an finish up the webgl renderer questions in this issue: #11

Thanks all!