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

[MIRROR] Remove several functions from collections.js which have ES5 equivalents #2957

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

Steals-The-PRs
Copy link
Collaborator

Mirrored on Nova: NovaSector/NovaSector#2054
Original PR: tgstation/tgstation#82417

About The Pull Request

This PR should not affect players: it purely serves to clean up some old functions mentioned in https://tgstation13.org/phpBB/viewtopic.php?p=724810.

In summary: Refactor of collections.ts and refactor of collection manipulation. The following paragraphs go into the nuances of the PR and reading it is optional.

I originally wanted to remove flow from fp.js, but I've decided against it until I have a more permanent solution for sortBy and uniq/uniqBy

common/collections.ts Refactor

  • sort(array) is the same.
  • sortStrings(array) has been replaced by sort(array).
  • Added types to reduce(...).
  • Restored old implementation of map(...) prior to #60961.
  • Rewrote most functions from the form f(...args)(array) to f(array, ...args).

ECMAScript 5 Array.prototype.* functions

Most of the functions have direct equivalents in ES5, but behavior varies slightly. Modern JS engines such as V8 and SpiderMonkey(I've not checked for IE) uses optimized objects for array functions such that typeof [].map(() => {}) is an object which is array-like, in contrast, the collection functions return an array.

All replacement functions are part of ES5(caniuse), which is supported natively by Internet Explorer version 10 and version 11(MSHTML version 6 and version 7 respectively).

collections.ts ES5
filter((element, index, array) => {})(array) array.filter((element, index, array) => {})
map(function)(array) array.map(function)
map((value, key, object) => {})(object) * Object.entries(object).map(([key. value], entries) => {})
filterMap(array, (element) => {}) array.map((element) => {}).filter((element) => element !== undefined)
reduce((accumulator, currentValue, currentIndex, array) => {}, initialValue)(array) array.reduce((accumulator, currentValue, currentIndex, array) => {}, initialValue)
zipWith((element1, element2, element3) => {})(array1, array2, array3) zip(array1, array2, array3).map(([element1, element2, element3]) => {})
sort(array) ** array.sort()
sortStrings(array) ** array.sort()

*: the second argument of the arrow function is Object.entries(object) rather than object.
**: there may be some differences in the way these functions sort(such as culture and casing considerations), but they should remain largely the same

Polyfills

Contrary to the statement on the original forum post, I've not decided to include Lodash because the project also encourages questionable practices. Lodash has a newer competitor, Radash, but I've decided against including that too as it's not mature enough to provide significant benefit over the functions defined in /tg/ui.

unique is a function present in core-js(a polyfill for https://github.com/tc39/proposal-array-unique), but I've decided against using it because it lacks TypeScript definitions, and the proposal has been in Stage 1 for quite a while.

Vectors

Rewrote vecAdd, vecSubtract, vecMultiply, and vecDivide. The following compares the current and new implementation of vecAdd.

Current Implementation:
Reduce followed by element-wise zip(zip and addition step combined, otherwise the diagram wouldn't fit)

$$ \begin{array}{c c c c} V_1 & V_2 & V_3 & V_4 \\ \begin{bmatrix} 1\\ 2\\ 3\end{bmatrix} & \begin{bmatrix} 4\\ 5\\ 6\end{bmatrix} & \begin{bmatrix} 7\\ 8\\ 9\end{bmatrix} & \begin{bmatrix} 10\\ 11\\ 12\end{bmatrix} \end{array} \to \begin{array}{c c c} V_{1 + 2} & V_3 & V_4 \\ \begin{bmatrix} 5\\ 7\\ 9\end{bmatrix} & \begin{bmatrix} 7\\ 8\\ 9\end{bmatrix} & \begin{bmatrix} 10\\ 11\\ 12\end{bmatrix} \end{array} \to \begin{array}{c c} V_{1 + 2 + 3} & V_4 \\ \begin{bmatrix} 12\\ 15\\ 18\end{bmatrix} & \begin{bmatrix} 10\\ 11\\ 12\end{bmatrix} \end{array} \to \begin{array}{c} V_R \\ \begin{bmatrix} 22\\ 26\\ 30\end{bmatrix} \end{array} $$

New Implementation:
Zip followed by element-wise reduce

$$ \begin{array}{c c c c} V_1 & V_2 & V_3 & V_4 \\ \begin{bmatrix} 1\\ 2\\ 3\end{bmatrix} & \begin{bmatrix} 4\\ 5\\ 6\end{bmatrix} & \begin{bmatrix} 7\\ 8\\ 9\end{bmatrix} & \begin{bmatrix} 10\\ 11\\ 12\end{bmatrix} \end{array} \to \begin{array}{c c c} Z_1 & Z_2 & Z_3\\ \begin{bmatrix} 1\\ 4\\ 7\\ 10\end{bmatrix} & \begin{bmatrix} 2\\ 5\\ 8\\ 11\end{bmatrix} & \begin{bmatrix} 3\\ 6\\ 9\\ 12\end{bmatrix} \end{array} \to \begin{array}{c} V_R \\ \begin{bmatrix} 1 + 4 + 7 + 10\\ 2 + 5 + 8 + 11\\ 3 + 6 + 9 + 12\\ \end{bmatrix} \end{array} \to \begin{array}{c} V_R \\ \begin{bmatrix} 22\\ 26\\ 30\\ \end{bmatrix} \end{array} $$


Fixed vecNormalize. Previously, it would attempt to divide a vector by a scalar(which is not a valid argument for vecDivide). The function has been corrected to properly each vector's component by the scalar.

Other Changes

  • Adjusted some types NtosNetDownloader.tsx for brevity.
  • Adjusted types around LibraryAdmin.tsx to fix type errors.
  • Removed fp.js' compose function.
  • Rewrote StackCrafting.tsx's filterRecipeList.

Why It's Good For The Game

Reduced code debt is good.

…equivalents (#2054)

* Remove several functions from collections.js which have ES5 equivalents (#82417)

* Remove several functions from collections.js which have ES5 equivalents

---------

Co-authored-by: Arthri <[email protected]>
@ReezeBL ReezeBL merged commit 5d411f4 into master Apr 19, 2024
27 checks passed
@ReezeBL ReezeBL deleted the upstream-mirror-2054 branch April 19, 2024 07:21
Iajret pushed a commit that referenced this pull request Jun 10, 2024
…DB IGNORE] (#2957)

* The Syndicate MODs are now resistant to batons knockdown. (#83510)

## About The Pull Request

Adds a new Module preinstalled into the nukie suits, and purchasable in
the uplink, grants resistance to batons knockdown.

## Why It's Good For The Game

At the last coderbus meeting, i personally asked to Fikou and Mothblocks
for more baton counters to be introduced, they seemed to be ok with the
idea, so here we are.

Introduces an unremovable module which grants to the wearer resistance
to baton knockdown, it comes preinstalled in all syndicate modsuits.

With the fix of "rest to prevent disarm", stun batons have gotten even
more powerful than they once were, and while i do agree secret kinda
abusable tech is bad for the health of the game, the end result is that
the stun meta is even more prominent than ever, so it's a good time
introduce a small counter to it.

From a thematic perspective, Syndie suits should at least make people
feel afraid, which is hard to do when a single prod from a stun stick
will make you drop like a sack of potatoes, lose your desword and get
slashed to bits.

On the gameplay side, wearing one of these will make you a target for
the entire station, if a traitor is willing to expose themselves, or in
the case of nukies, forced to; they should probably have a small edge on
what's the most common tool used to stop them.

Keep in mind that this only prevents the knockdown, it does nothing for
the stamina damage, you'll still get stamcritted in a couple of hits,
nothing changes on that front.

This is merely to prevent a scenario where a single baton hit will
immediately lose you a fight.

## Changelog

:cl:
add: adds the MOD shock-absorption module, into the game.
add: The MOD shock-absorption module into the the uplinks, costs 4 TC.
balance: Nukie modsuits come with the shock_absorption module
preinstalled.
/:cl:

* The Syndicate MODs are now resistant to batons knockdown.

---------

Co-authored-by: EnterTheJake <[email protected]>
Co-authored-by: NovaBot13 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants