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

Define PLATFORM and PLATFORM_NUMBER based on the minimum Android SDK Version. #1826

Closed
wants to merge 13 commits into from

Conversation

MAJigsaw77
Copy link
Contributor

This PR updates PLATFORM and adds PLATFORM_NUMBER to match with the project's minimum Android SDK version, moving away from the previous hardcoding of android-21.

The main thread can easily look these up by ID, and in HTML5, sending the full `JobData` can cause errors.
`__activeThreads` and `__idleThreads` only need to be allocated for multi-threaded pools. Plus, there's no benefit to using a `List` here; we only add to and remove from the end.

And finally, checking `event.job == null` instead of `isOfType()` is faster and avoids an issue in HTML5. Sadly it is less safe, so we might need to revisit it eventually.
@MAJigsaw77
Copy link
Contributor Author

@player-03 May i get a review on this pr?

player-03 and others added 7 commits August 15, 2024 15:39
It looks like we'll want to take `BackgroundWorker` in a different direction, so for the moment it's safest not to change anything about it. That way, there's only one historical version to maintain backwards compatibility with.
As with `BackgroundWorker`, we're postponing major changes to give us more time to consider.
While I put a lot of effort into that guide, we're changing several things suddenly, and I don't have time to make sure it's up to date.
Restore old `Future` and `BackgroundWorker` behavior.
@dimensionscape
Copy link
Member

Does this break any existing workflow?

@MAJigsaw77
Copy link
Contributor Author

Does this break any existing workflow?

I don't think it would break anything. I've tested this on a project and it works just fine.

@dimensionscape
Copy link
Member

Does this break any existing workflow?

I don't think it would break anything. I've tested this on a project and it works just fine.

Wont developers need to include these in their project xml now?

@MAJigsaw77
Copy link
Contributor Author

MAJigsaw77 commented Aug 16, 2024

Does this break any existing workflow?

I don't think it would break anything. I've tested this on a project and it works just fine.

Wont developers need to include these in their project xml now?

Noup, using project.config.getInt("android.minimum-sdk-version", 21); makes it so 21 is the default value if theres no version found within project.xml

@dimensionscape
Copy link
Member

dimensionscape commented Aug 16, 2024

Sounds good. PR to the correct 8.3 branch though.

@MAJigsaw77 MAJigsaw77 changed the base branch from develop to 8.3.0-Dev August 16, 2024 21:34
@MAJigsaw77
Copy link
Contributor Author

I changed the base branch to 8.3.0-Dev.

@MAJigsaw77
Copy link
Contributor Author

Uh, i think i broke something, I'll close this and change the branch

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.

5 participants