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

3.1.51 #1501

Merged
merged 135 commits into from
Oct 21, 2024
Merged

3.1.51 #1501

merged 135 commits into from
Oct 21, 2024

Conversation

ebkr
Copy link
Owner

@ebkr ebkr commented Oct 21, 2024

No description provided.

anttimaki and others added 30 commits September 13, 2024 16:23
The date fields denoting creation and last update date are in fact
strings, not dates. IDK if it has always been this way or if e.g.
moving the online mod list from disk to IndexedDB changed this.

Luckily the date fields are only used for newer/older than comparisons,
and the ISO 8601 date strings yield the same results for those
operations as the Date objects would.
Online mod list was watching a wrong variable, and wouldn't fire an
autoupdate if the main mod list changed, i.e. if there was a background
update from Thunderstore API while user had the online mod list open.

This was broken some 6 months ago by yours truly while moving the mod
list into Vuex store.
Fix ThunderstoreMod date field typing
Fix watching online mod list for changes
GitHub actions dropped the support for MacOS 11, causing all CI jobs to
fail. Updating to MacOS 12 would require updating 3rd party
dependencies, namely electron-builder (see PR #1409).

MacOS isn't officially supported by this project, and it was added to
CI pipeline just because doing so was easy and only required a few
lines of config code. Now keeping MacOS on the CI pipeline would
require testing that updating electron-builder has no ill side-effects.
That work seems to have a low return of investment, so changes are it
won't get done anytime soon.
Add Hard Time III
Add Paquerette Down the Bunburrows
Add Shapez 2
…mponent

Separate Zip extraction function from Profile Import Ui component
Distance
FNAF Into The Pit
Tank Team
Separate parseYamlToExportFormat function from ImportProfileModal UI …
…1385)

Additionally this avoids some internally inconsistent states, e.g. when the
game is changed but the mod list still contains mods for the old game.

---------

Co-authored-by: Antti Mäki <[email protected]>
Thunderstore API uses blob files to serve immutable data, as this can
be efficiently cached via CDN. The blobs contain stringified JSON that
is compressed with gzip to save storage space.
Fetching Lethal Company's package list containing 30k+ mods in one
request has been pushing the limits for a while now. For users with
slower connections the request timeouts, and the ICP message size limit
is soon starting to cause problems.

To address these problems, the package list is now downloaded in
chunks. Each chunk is a separate blob file that can be heavily cached
on CDN level. Chunks are downloaded serially to support slower
connections. Caching to IndexedDB is done only when all chunks have
been downloaded so transaction can used to make the operation atomic.
The hash identifies the last seen package list index, allowing us to
skip updating the database from file cache if there's no changes to
the package list.

The new table contains 0..1 rows for each game (because community
identifiers aren't available on the mod manager currently). Internal
game identifier acts as the primary key to prevent duplicates. Compound
index of game+hash is added for performance, and to silence "warnings"
that Dexie would otherwise log into the console.

Dexie's put method, i.e. upsert, is used to update the hash to the
database.
The hashes will be used to identify previously seen package list
indices. The Thunderstore API uses the same hash to identify blobs.
The hash is recalculated on the mod manager because:

- The API returns a redirect response, and we can't access the headers
  of the original response, only the result of redirect from the CDN
- The redirect can't be prevented and handled manually in the browser
  environment
- The response from the CDN does contain the hash as its used in the
  file name, but the only place Axios stores it in the response is a
  field that looks like it could easily change in the future versions

Since hashing the amount of data we're handling should be in a scale of
single digit milliseconds per chunk, this shouldn't be a problem.
While this makes some parts of the code (updating the progress text and
percentage shown on the UI) harder to follow, I feel this improves the
readability of the code overall. Especially when the upcoming additions
to the flow are taken into account.
In SplashMixin, skip downloading the chunks and updating them into the
IndexedDB if the index file indicates the packages haven't changed. If
downloading the index file itself fails, try to load earlier mod list
from the IndexedDB like it did earlier.

In UtilityMixin, skip the whole process if the index file indicates the
packages haven't changed, as at this point we already have the mod list
loaded in to Vuex. If loading the index file itself fails, the error is
caught by the caller, which handles retries as it did earlier.
Old approach used to read the timestamp from the package list table
since it wasn't available elsewhere. This approach no longer works
when we don't update the package list into database if it didn't
change. Instead, store the information in the same table as the
community's last seen index chunk hash. This timestamp is updated
whenever the same index hash is seen again, or when the hash changes
AND the new package list is successfully written to DB.

Old approach loaded the timestamp into Vuex when the package list was
read to Vuex state. With the new approach we need to take into account
that UtilityMixin skips updating the package list in memory if the list
didn't change. Instead, we need to update the timestamp manually.
While r2modman used Node's zlib as intended, TSMM used a library that
happened to be present as a 2nd degree dependency. While it worked, the
library had poor performance, taking ~3s to decompress each chunk,
while zlib and fflate takes next to no time at all.

Using DecompressionStream instead was considered, but based on a single
google result it seems that would require updating the TypeScript
version, which is a side quest for another time.

It was also considered sniffing the app version and using fflate only
on TSMM, so it wouldn't need to be added as a dependency on r2modman.
In the end gut feeling decided that would be unnecessarily complex and
prone to break due to future changes.
Thunderstore's main CDN regularly just fails to respond to requests.
Now that package list chunks are downloaded from the CDN, the issue
also affects this part of the program. While the only real fix is to
migrate to more reliable service provider, ETA on that is unknown.
Meanwhile do what can be done to mitigate the issue.

- Increase the number of retries and reduce the interval between them
  when loading the index chunk. Sometimes spamming the same request
  makes the CDN "wake up" and serve the file
- Add "preferred CDN" query parameter to index chunk request. If the
  Thunderstore API endpoint supports the received value, it will uses
  that CDN when returning the redirect response. Intercepting the
  redirect response to manipulate the redirect URL in the browser
  environment has proven hacky in the past and is avoided here
- Change the CDN on the fly on client when requesting the package list
  chunks. Since these request point directly to the CDN rather than
  Thunderstore API, manipulating them directly is feasible.
Separate installModAfterDownload function from UI component
anttimaki and others added 28 commits October 16, 2024 16:13
Discard profile if importing fails midway through the process
The helper is used to convert an array of dependency strings to
ThunderstoreCombo objects using data from the IndexedDB. This is a
preliminary step for changes to reduce the need to have a full list of
community's mods and their versions in memory at all times.

read the data required to build an arrayy dependency tree
(well, an array at least
Reading the data from DB allows eventually dropping the reference to
whole list of ThunderstoreMods passed as an argument.

Unfortunately this requires bringing in a dependency to GameManager.
It's required currently to know which community's package list should
be used to look for the mods. Ideally the packages and listings would
be separated, but that's outside of the scope of the current task.
Additionally it would change the behaviour, as currently
inter-community dependencies aren't allowed.
…edDB

Reading the data from DB allows eventually dropping the reference to
whole list of ThunderstoreMods passed as an argument.

Unfortunately this requires bringing in a dependency to GameManager.
It's required currently to know which community's package list should
be used to look for the mods. Ideally the packages and listings would
be separated, but that's outside of the scope of the current task.
Additionally it would change the behaviour, as currently
inter-community dependencies aren't allowed.
None of the provider's methods no longer need to have access to full
list of mods available on Thunderstore API. The same data is read
directly from IndexedDB when it's needed.
The result array was both returned from the function, but also mutated
in place. I assume this is done to avoid having to create multiple
copies when the callstack gets deep. Stop returning the array to make
it clear it's used by reference.
Default parameters are considered harmful as someone might forget to
use them. Required Enum parameters are considered better for
readability.
Check the existence of the target package and version directly from the
IndexedDB cache. After the change the Manager component no longer needs
access to the complete package list of the selected community.
The recently added helper is more suitable for the use case. This also
means the implementation doesn't depend on ThunderstoreMod containing
references to all related ThunderstoreVersions anymore - a change we
want to introduce to reduce the apps memory footprint.
Set the default values in the beginning and overwrite them only if
suitable overwrite values are found.
Do not depend on ThunderstoreMod containing information about all the
related ThunderstoreVersions, as the connection will be severed in an
attempt to reduce memory footprint of the app.
Do not depend on ThunderstoreMod containing information about all the
related ThunderstoreVersions, as the connection will be severed in an
attempt to reduce memory footprint of the app.
This hides the latest version's data behind a method so we can change
the internal implementation of how ThunderstoreMod handles version
information.
Having the method only in one store module makes refactoring it easier.
ProfileModule is more natural place since it tracks the list of locally
installed mods, and modsWithUpdates answers the question of which of
those mods are outdated.
Synchronous getter will now return an array of ThunderstoreMods rather
than ThunderstoreCombos. This way the function can stay synchronous
even when ThunderstoreMods no longer contain reference to
ThunderstoreVersions. ThunderstoreMod is sufficient to most use cases -
in fact all but one only care about the number of returned items and
therefore required no changes in call sites.

Asynchronous action returns an array of ThunderstoreCombos to the only
place where the combos are actually needed.
As far as I can tell, the conversion isn't really needed. The doc
strings of ReactiveObjectConverterInterface claims that access to class
methods is lost, but based on testing this is not true. Another claim
is that this grants access to object's private fields. The fields can
be accessed, but same is true to any ThunderstoreMod object, even if
they aren't passed through Vue's methods. Aside from getting a clear
TypeScript warning about accessing a private field in IDE (compile
time) nothing prevents accessing the field in JS files (run time).

Ideally this might speed up OnlineModList rendering, as the unnecessary
object creation is skipped.
Commonly used version-specific data - latest version number, icon and
description are now stored in the ThunderstoreMod object. Rest of the
version info needs to be fetched separately where it's actually used.

This change drastically reduces memory usage for larger communities
like Lethal Company.
…ovider

Decouple download provider from full list of ThunderstoreMod objects
…Protocol

Don't require complete package list to install mods via URL protocol
Use getCombosByDependencyStrings in ProfileUtils
…dModal

Don't depend on ThunderstoreMod containing all ThunderstoreVersions in components
Don't depend on ThunderstoreMod containing all ThunderstoreVersions in Vuex
…reMod

Drop ThunderstoreVersion references from ThunderstoreMod
Add TCG Card Shop Simulator
Updated versions and CHANGELOG
@ebkr ebkr merged commit 1d9162a into master Oct 21, 2024
6 checks passed
public static async fromProtocol(protocol: string, game: Game): Promise<ThunderstoreCombo | R2Error> {
// Remove protocol information and trailing slash, leaving the package version information.
const reducedProtocol = protocol
.replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\.)?thunderstore\.io/'), '')

Check failure

Code scanning / CodeQL

Useless regular-expression character escape High

The escape sequence '\.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a
regular expression
.

Copilot Autofix AI 27 days ago

To fix the problem, we need to ensure that the regular expression correctly matches a literal dot character. This can be achieved by using double backslashes (\\.) in the string literal to ensure that the backslash is correctly interpreted in the regular expression.

  • Update the regular expression on line 15 to use double backslashes (\\.) instead of a single backslash (\.).
  • This change ensures that the regular expression matches a literal dot character as intended.
Suggested changeset 1
src/model/ThunderstoreCombo.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/model/ThunderstoreCombo.ts b/src/model/ThunderstoreCombo.ts
--- a/src/model/ThunderstoreCombo.ts
+++ b/src/model/ThunderstoreCombo.ts
@@ -14,3 +14,3 @@
         const reducedProtocol = protocol
-            .replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\.)?thunderstore\.io/'), '')
+            .replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\\.)?thunderstore\\.io/'), '')
             .replace(new RegExp('\/$'), '');
EOF
@@ -14,3 +14,3 @@
const reducedProtocol = protocol
.replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\.)?thunderstore\.io/'), '')
.replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\\.)?thunderstore\\.io/'), '')
.replace(new RegExp('\/$'), '');
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
public static async fromProtocol(protocol: string, game: Game): Promise<ThunderstoreCombo | R2Error> {
// Remove protocol information and trailing slash, leaving the package version information.
const reducedProtocol = protocol
.replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\.)?thunderstore\.io/'), '')

Check failure

Code scanning / CodeQL

Useless regular-expression character escape High

The escape sequence '\.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a
regular expression
.

Copilot Autofix AI 27 days ago

To fix the problem, we need to ensure that the period character in the regular expression is correctly escaped. This can be done by adding an extra backslash in the string literal, so that the regular expression engine interprets it as a literal period.

  • In general terms, the problem can be fixed by ensuring that the backslash is correctly escaped in the string literal.
  • Specifically, we need to change the regular expression on line 15 to use \\. instead of \..
  • The change should be made in the file src/model/ThunderstoreCombo.ts on line 15.
Suggested changeset 1
src/model/ThunderstoreCombo.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/model/ThunderstoreCombo.ts b/src/model/ThunderstoreCombo.ts
--- a/src/model/ThunderstoreCombo.ts
+++ b/src/model/ThunderstoreCombo.ts
@@ -14,3 +14,3 @@
         const reducedProtocol = protocol
-            .replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\.)?thunderstore\.io/'), '')
+            .replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\\.)?thunderstore\\.io/'), '')
             .replace(new RegExp('\/$'), '');
EOF
@@ -14,3 +14,3 @@
const reducedProtocol = protocol
.replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\.)?thunderstore\.io/'), '')
.replace(new RegExp('ror2mm://v1/install/([a-zA-Z0-9]+\\.)?thunderstore\\.io/'), '')
.replace(new RegExp('\/$'), '');
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
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