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

Loosen platform compatibility checks #240

Closed
wants to merge 1 commit into from

Conversation

tehbarney86
Copy link

@tehbarney86 tehbarney86 commented Jan 18, 2025

Proposed Changes

This pull request updates the platform checking functionality in TW so that it checks if TW's platform name is included in a given project's platform name.

Reason for Changes

The current platform checking logic in TW can lead to incompatibility issues with other platforms that include the TW name in their platform name, (e.g. cross-compilers and other addons that are compatible with TW and other Scratch modifications). To address this issue and allow for broader compatibility, the logic has been updated to check if the 'TurboWarp' (runtime.platform.name) string is included in the platform name, rather than strictly matching the name exactly. This improves compatibility without compromising the integrity of the platform checking functionality, reminiscent of useragents in the http/https protocol.

Test Coverage

As this pull request involves a simple change in logic from using === to includes(), no tests were required to verify the behaviour of the change. This is because the test coverage of this particular area was already sufficient. The string.includes() function is standard JavaScript, therefore testing isn't mandatory, nor should be. (...at least I hope so)

TL;DR

remove annoying pop-up if platform name is not exactly 'TurboWarp' but addon is compatible with it

@WlodekM
Copy link

WlodekM commented Jan 18, 2025

omg what a coincidence this helps with my project how could that be
anyways this is awesomesauce because someone could make their thingy add something like TurboWarp-compatible to the platform name in the meta and it wouldn't yell at you that the platform name isn't exactly TurboWarp

@tehbarney86

This comment was marked as spam.

@GarboMuffin
Copy link
Member

GarboMuffin commented Jan 18, 2025

What mod is this for?

This is the wrong approach and it absolutely does need tests but I want to know about your usecase first

@GarboMuffin GarboMuffin changed the title addon compatibility Loosen platform compatibility checks Jan 18, 2025
@GarboMuffin
Copy link
Member

All the warning says is that

Compatibility with TurboWarp is not guaranteed. You can continue at your own risk, but we may not be able to help if you encounter any problems.

I'm inclined to say that this is perfectly reasonable to say for any third-party platform? that's why I want to know what mod wants this

@tehbarney86
Copy link
Author

All the warning says is that

Compatibility with TurboWarp is not guaranteed. You can continue at your own risk, but we may not be able to help if you encounter any problems.

I'm inclined to say that this is perfectly reasonable to say for any third-party platform?

The warning is true, it's just that some addons can be compatible with TurboWarp, and have the right to specify if they are or not by specifying it in their platform name, instead of force-making it compatible it with TurboWarp only.

What mod is this for?

This addon is still indev and very barbones, but as an example, WlodekM's SLTLCC is a cross-compiler which works by converting (and in future injecting) text-based code to scratch projects compatible with any scratch mods.

This is still 'as an example', and not just for one mod to "slide through".

This is the wrong approach and it absolutely does need tests but I want to know about your usecase first

Forcing the authors of projects created in addons like these to be compatible with only one platform (keep in mind this is only in example, who knows, but there is no denying that such 'addons' have been in the works or even already finished off and somewhere laying around on github for long now) is just an unnecessary evil, the only available road through this with full compatibility in mind is just compiling the same exact project for every platform with just changing one variable - the project's platform name.

The question is not "why", it's "why not" (and hold on for a second here, this is not some cave johnson quote where i fire a man for being in a wheelchair), people should have freedom in making their projects compatible with any scratch mod. Take example the http's useragent string, people can specify whether it's some SLTLCC thing some 14yo ukrainian is yapping about, as a challenge of sorts (there's lots of reasons why people are still trying to run doom on everything they can) or just in sake of flexibility for everything there is, not by just making the one and only standard the original Scratch 3.0/2.0.

after all, it's literally just ~20 bytes of change lmao, no need to be this overdramatic for just changing === to .includes(), this is not a security oversight whatsoever and will not harm tw in any way

@GarboMuffin
Copy link
Member

GarboMuffin commented Jan 18, 2025

If the projects being generated work perfectly fine in all mods by just changing the platform name, then it sounds like it is only using features in vanilla Scratch. In this case you can exclude meta.platform entirely and not see any warning since that is interpreted as the platform being vanilla Scratch.

@GarboMuffin
Copy link
Member

And then you can store whatever metadata you want in meta.agent

@GarboMuffin
Copy link
Member

GarboMuffin commented Jan 18, 2025

You definitely do not want to cite the modern mess that is User-Agent on the web as inspiration for a new feature. It is a disaster that harms compatibility, not helps it. And anyways meta.agent is literally the spot to store the equivalent if you really want to.

@GarboMuffin
Copy link
Member

GarboMuffin commented Jan 18, 2025

you can think of meta.platform.name as being the "CPU architecture" that the project is supposed to run in

since you are making a project generator (analogous to a compiler like gcc or clang), not some new project runner with new blocks and behaviors (analogous to adding new CPU instructions), you shouldn't be inventing new meta.platform.name values

@GarboMuffin
Copy link
Member

We will change the unknown platform warning to say something along "the project was made for another platform" instead of "the project was made with another platform" to reflect the above

@GarboMuffin
Copy link
Member

If you have a scenario where being compatible with multiple non-vanilla-Scratch platforms simultaneously is necessary we can reopen this with a different design that isn't simple string inclusion

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

Successfully merging this pull request may close these issues.

3 participants