Skip to content
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

Remove NO_LONGER_USED_FLAG code #395

Open
liammulh opened this issue Apr 17, 2023 · 4 comments
Open

Remove NO_LONGER_USED_FLAG code #395

liammulh opened this issue Apr 17, 2023 · 4 comments

Comments

@liammulh
Copy link
Member

The single source of truth for a sim's strings should be the strings in the published sim.

It's been a while since I've dealt with the code that uses the NO_LONGER_USED_FLAG constant. I want to know what the scenario it covers is, and whether it's necessary to cover that scenario.

@liammulh
Copy link
Member Author

liammulh commented May 1, 2023

@jbphet and I looked at the NO_LONGER_USED code, and we think it can be removed. We are going to do an informal regression test by:

  • Logging translation report objects in zh_TW before removal
  • Removing NO_LONGER_USED code
  • Logging translation report objects in zh_TW after removal
  • Check translation report objects before and after for discrepancies

If there are no discrepancies, I think we should remove the code.

@liammulh liammulh changed the title Determine whether we need NO_LONGER_USED_FLAG Remove NO_LONGER_USED_FLAG code May 5, 2023
@liammulh
Copy link
Member Author

liammulh commented May 5, 2023

Okay, I just removed the NO_LONGER_USED_FLAG, and I can confidently say it is necessary. It messed up just about every stat. However, I am confused as to why this is the case.

stats

@liammulh
Copy link
Member Author

liammulh commented May 5, 2023

I am going to defer further investigation and prioritize other tasks for now.

@liammulh
Copy link
Member Author

liammulh commented May 5, 2023

@jbphet and I figured out that we do need this code. We added a comment above the constant:

// Define a value that indicates that a string is no longer used. It's
// possible that a sim could have a string "no longer used", so I
// (Liam Mulhall) added a nonsense word "gooble". The primary purpose of
// this constant is to strip out strings that were once used for accessibility like
// SCENERY_PHET/ResetAllButton.name or JOIST/HomeButton.name. These
// strings are present in some older sim publications, but not in the
// master version of their string file(s). Now, all accessibility strings
// are prefixed with a11y.

If someone accidentally removes a string from the English master string file for e.g. joist, then the string won't be presented to the user for translation, and that string won't be considered in the translation statistics. This is an edge case, but something to keep in mind if a translator writes in about not being able to translate a string.

If at some point we re-publish all sims off of master, then this code will be obsolete, and can be removed. At this point, it still serves a purpose, so we are deferring this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant