core: Partially implement system.capability #10469
Conversation
There was a problem hiding this comment.
Thanks for your effort, and welcome!
This is great -- my only potential concern is breaking content that could be doing simple checks for Windows, etc.. Possibly we want to default to some standard appropriate settings (Windows XP, etc.) instead of what the actual system is, but I'm not sure. Tagging @n0samu because he probably has a better idea of which content might actively depend on these settings.
Eventually we could make this all configurable, via the JS config on web or via the UI when we get around to adding that on desktop.
|
Thank you! :) I also planned on adding CLI arguments to make the Player and system.capability attributes configurable, but I thought that it might make sense to do that in a different pull request and add these changes first. I could do this with the JS config as well, as you suggested. I personally think that it would be best if Ruffle correctly emulated the flash player and just returned exactly what it would return, especially since some flash games might contain easter eggs depending on the OS and such. |
|
Sorry for the delay in getting back to you, thanks for the patience! The only popular content @n0samu mentioned was Fancy Pants 4 which would show an error message for Linux OS (assuming it was Android), so this is already an improvement there (current Ruffle only returns "Linux"). I'm sure there's other OS-dependent content lurking out there, but I agree that this can be taken care of by configuration settings -- it'd be up to the user/website owner to configure Ruffle appropriately to spoof the OS. Thanks for the well-commented code If you'd like to rebase, I'll go ahead and merge! |
|
Thanks! And yes, I'll rebase the branch onto the current master branch now. I prefer rebasing it by myself instead of using the Github feature (e.g. because I prefer my commits to be verified), so I appreciate it if I can do it this way :) |
Korne127
force-pushed
the
master
branch
3 times, most recently
from
f69ec7d to
b0221cd
Compare
Lord-McSweeney
added
the
waiting-on-review
|
Sorry for the delay in getting to this. Could you rebase and resolve the merge conflicts? |
The SystemCapabilities bitmap contains the flash api system.capabilities booleans, which are currently not implemented. Previously, the bitmap was empty, meaning that all booleans were set to false. Now, the bitmap values have been stubbed out. If clauses setting the respective booleans have been added. The actual tests aren't implemented, but the stubs are filling the bitmap with probable entries (e.g. most systems have audio encoding, therefore the audio encoding boolean is now set to true). This allows the capabilities to be more accurate (while not being implemented yet) and simplifies the actual implementing, since only the conditions of the stubs need to be replaced.
A RuffleType enum has been added. It has the possible values DesktopPlayer and WebPlayer. The Player now has a variable ruffle_type whose type is this enum. The PlayerBuilder sets the ruffle_type of a Player via conditional compilation. The SystemProperties struct now receives the ruffle_type at its creation. It uses it to implement system.capabilities.playerType. The playerType is set to StandAlone if the desktop player is used and to PlugIn if the web player is used. Closes ruffle-rs#8309.
The dependency sys-locale has been added. The SystemProperties constructor now uses it to get the locale of the current system and set the language variable correspondingly. A new language_region variable has been added to the SystemProperties struct. This is because the language string can contain a custom region part as well, which can’t be saved in the language enum. (This custom region part has previously not been stubbed.) The SystemProperties constructor sets this variable as well. Since this variable is dependent on the flash player version, the SystemProperties struct now receives the flash player version at its creation. Since the language string has been separated into two variables, the way of getting the language string has changed. A new function get_language_string has been added; it builds and returns the language string out of the language and language_region variables. (The previously used function has been replaced by a display format implementation used by the new function.) system.capabilities.language has only been partially implemented because the locale language (retrieved by sys-locale) is always used. However, according to the flash specs, the UI language should be used if the OS is Windows and the flash player version >= 7. This is not yet implemented. Minor style improvements have been added.
The SystemProperty constructor now recognises the OS and sets the os variable. It uses the new get_os helper function, which has multiple versions using conditional compilation. If a desktop version is used, it gets the OS using the environment constant. If Windows is used, it determines the Windows version using the dependency os_info. If that's not possible, the ver CLI command is used and the result parsed instead. The Windows version then gets matched to one of the possible os values. If the web version is used, it gets the user agent using the dependency web-sys. The user agent is then used to determine the OS and the variable is set accordingly. Co-authored-by: bestlinuxgamers <52172848+bestlinuxgamers@users.noreply.github.com>
The SystemProperty constructor now recognises the CPU architecture and sets the cpu_architecture variable. It uses the new get_cpu_architecture helper function, which has multiple versions using conditional compilation. If a desktop version is used, it gets the CPU architecture using the environment constant. If the web version is used, it gets the user agent string using the dependency web-sys. The user agent is then used to determine the CPU architecture. That is a rudimentary implementation since most user agents don't allow to infer the architecture, but it seems that there is no more reliable way.
The variable system.capabilities.cpuAddressSize existing in AVM1 has previously not been stubbed. It has now been added and implemented. The SystemProperty constructor determines the address size by evaluating the size of a pointer and sets the cpu_address_size variable to the values 32 or 64.
The server string varies depending on the flash player version (since new variables have been added in later flash player versions). The server string has previously not returned the result of the latest flash player version (32) and not included all variables. This has been fixed and all missing variables have been added. Since the server string is usually parsed, the additional variables (if emulating older flash player versions) shouldn't be a big problem, but it might still be better to return the proper string of each flash player version. Additionally, a mistake has been fixed that system.capability.hasAccessibility has been inverted before including it in the server string.
The stubbing of system.capabilities.os has previously not included some possible return values. These have been added and implemented. Older flash player versions don't recognise newer systems. This is currently not implemented.
Several minor improvements and fixes have been added. The manufacturer string (system.capabilities.manufacturer) depends on the SWF version. Previously, the flash player version has been used instead though, resulting in a wrong string. This bug has been fixed. The manufacturer string and the server string (which contains the manufacturer string) are now generated with the SWF version. Several comments have been added to improve documentation of what changed between flash player versions and / or SWF versions (and is not correctly emulated yet). The SystemCapabilities entries have been commented. Todos have been fixed and added to mark what still needs to be implemented.
Previously, system.capabilities.language has always used the locale on Windows. However, according to the flash specs, the UI language should be used on Windows if the flash player version is >= 7. This has now been implemented. If the OS is Windows and the flash player version >= 7, the UI language is now retrieved via the Windows API (using the winapi crate) and used instead of the locale. Additionally, the language string is now lowercased before parsing and converting it into the language variables. This way, the language detection still works if an OS returns the language string in uppercase. Co-authored-by: bestlinuxgamers <52172848+bestlinuxgamers@users.noreply.github.com>
The suggested changes to the system.capabilities implementation in the feedback to the pull request have been implemented. Therefore, this commit consists of multiple smaller changes: - The Windows versions before (including) Windows XP aren't detected anymore. This is because Rust doesn't compile for those versions, therefore Ruffle can't run on them and the code is unnecessary. - The failsafe using the ver CLI command and parsing its result if os_info can't determine the Windows version has been removed, because it has only been used for Windows versions before Windows 2000, which os_info can't determine. - The enum values of the RuffleType enum have been renamed for brevity. - The formatting has been improved.
| #[cfg(target_family = "wasm")] | ||
| fn get_os() -> OperatingSystem { | ||
| let window = web_sys::window().unwrap(); | ||
| let user_agent = window.navigator().user_agent().unwrap(); |
There was a problem hiding this comment.
Would getting the user agent/operating system be better moved to an implementation on the navigator backend?
There was a problem hiding this comment.
Yeah, I see that point (although ExternalNavigatorBackend, TestNavigatorBackend and NullNavigatorBackend would share the same implementation (but this could easily be done with a function all three call)).
Do you prefer this? Then I can change it like that :)
@Aaron1011 (I hope the ping is okay) what do you think about this?
There was a problem hiding this comment.
I don't have a strong preference either way.
There was a problem hiding this comment.
Hey @Lord-McSweeney,
I tried changing it to the NavigatorBackend calculating the OS, but that's not possible in the current structure because the OperatingSystem is inside several nested private modules. And moving it would be pretty inconsistent to all the other property enums.
I think it might make more sense to take a general approach to re-use AVM1 system.properties code in AVM2 because there is a big overlap (most AVM1 properties are also available in AVM2); maybe e.g. using a shared set of functions calculating these values when the properties structs are built.
|
FWIW I'm with @Herschel in that
|
danielhjacobs
added
A-avm1
This pull request implements system.capability attributes, and fixes and improves the stubbing.
Additions / Implementations:
Fixes:
Stubbing:
Other:
This pull request should close #8309, since it implements system.capabilities.playerType.
It should also check the properties listed here in #278 as fully implemented. The other properties shouldn't be checked, since they're not actually implemented yet and need to be worked on.