Closed GabrielBoninUnity closed 9 months ago
there's an oversight regarding the UnityPlayer.so for the Linux player
Not so much, as we only need to detect any version for the whole game, and pretty much all games on Steam support Windows.
Can (?:^|/)UnityEngine\..+$
be removed given the other regexes? And what about (?:^|/)Assembly-CSharp\.dll$
Thank you for your Feedback xPaw
Regarding your question if (?:^|/)UnityEngine..+$ should be removed. Yes, (?:^|/)UnityEngine..+$ looks redundant since “UnityEngine” is already in the other regex (?:^|/)Unity(?:Engine|Player).(?:dylib|dll)$ . Note that we would not recommend expanding UnityPlayer since it could produce false positives like UnityPlayer.sod. If you want UnityEngine expanded, it’d be better to match UnityEngine.XXXXModule.dll for example.
For the regex (?:^|/)Assembly-CSharp.dll$ . We would say that it is probably better to keep it for legacy reasons. Games that are made made by using Mono scripting backend or old games will have this file.
Before sending you a revised version of the code, I would like to get your feedback on that. What do you think? Thanks Gabriel
Regarding your question if (?:^|/)UnityEngine..+$ should be removed.
Remove it.
For the regex (?:^|/)Assembly-CSharp.dll$ . We would say that it is probably better to keep it for legacy reasons.
Will these legacy games not match the other two regexes?
Great, I will remove it.
For the regex (?:^|/)Assembly-CSharp.dll$, I can't point to examples off the top of my head but there are cases where they might not. But we are talking about truly old games (7 years or older), and I'm not sure whether your algorithm ever rescans old games. It would be interesting to see if removing that rule makes it not detect some games anymore. Again, let me know what is your opinion about that.
I'm not sure whether your algorithm ever rescans old games.
Every rules change is a full rescan.
And it looks like it's safe to remove these two regexes after a quick test.
Resume of the changes after discussion:
For the rule.ini: I have removed the (?:^|/)UnityEngine..+$ as requested to remove redundancy since the other regex already picked up these files. Also removed (?:^|/)Assembly-CSharp.dll$ since xPaw confirmed it looks safe to remove after our previous discussion
For the tests/types/Engine.Unity.txt:
Remove these since the regex has been removed.(Were also duplicates in the file) /UnityEngine.@ /UnityEngine.@@
We don’t need to test for scenarios like UnityEngine.XXXXModule.dll. UnityEngine.dll will always be present if that one is present. Sub/Folder/UnityEngine.CoreModule.dll UnityEngine.CoreModule.dll
After running php tests/Test.php --> All tests have passed.
Let me know if this fits you. Thanks Gabriel
First off, we're truly impressed by this project and appreciate the opportunity to contribute. My name is Gabriel, and I represent the engineering team at Unity. We've taken a close look at the regex utilized for identifying the Unity engine within your project and believe there are opportunities for enhancement.
Summary of Changes 1) The existing regex pattern Unity[] = (?:^|/)globalgamemanagers.assets$ is effective for engine versions up to 2018.x. However, it defaults to using UnityEngine.dll for these versions, which is only present when the game employs the [Mono scripting backend], a specific project setting. Consequently, games built on older Unity versions with the IL2CPP scripting backend won't be recognized by this regex. To address this, we suggest detecting such games through the presence of a/globalgamemanagers.assets file.
2)In the regex pattern Unity[] = (?:^|/)Unity(?:Engine|Player).(?:dylib|dll|so)$, there's an oversight regarding the UnityPlayer.so for the Linux player, which is why we've introduced the |so inclusion.
We hope these adjustments will further refine the already impressive algorithm. Thank you for considering our input.