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

Fix time unit of API handling and refresh panel materials in time #1883

Merged
merged 5 commits into from
Mar 18, 2017

Conversation

sinsinpub
Copy link
Contributor

@sinsinpub sinsinpub commented Mar 8, 2017

As there are many codes changed, open a PR for more tests to check if I missed something or not.

No feature changed, just unify and refine time handling codes when timestamp converted to seconds or hours, and add time unit to vars name.

At first, my motivation is the fix thingy. After double checking of each API handling, I have to clarify and explain the enhancement of feature: Keep tracking with game displaying for 8 basic materials (fuel, ammo, steel, bauxite, torch, bucket, devmat, screw), show their latest values in time and correctly at devtools panel.

Long story: as we know, there are 3 types of API call which would consume or gain materials:

  1. In api_data of response, there is api_materials liked data to tell us all the absoluate values of materials the player hold.
    we have used these values as resources cached in Player.lastMaterial array and record them into IndexDB every hours.
    API examples of this type: api_port/port, api_get_member/material, api_req_hokyu/charge, ...
  2. There is no materials data in API request or response, but we (KC client side) are able to infer the increasement or decresement values from other API attributes or known shipgirl data.
    we have recorded these changes into IndexDB for ledger, but do nothing with the Player.lastMaterial cache array.
    API examples: api_req_kaisou/remodeling, api_req_kousyou/createship, api_req_quest/clearitemget, api_req_nyukyo/start, api_req_mission/result, api_req_air_corps/set_plane, ...
  3. No materials data in API and only KC server side update the exact values of resources.
    we can only try to compute them by ourselves.
    This type of API is mainly new game mechanism about LBAS and jet bombers such as: api_req_map/start_air_base

This PR also done:
Check all the API handlers, of type 2 above, add resources and consumables updating and refresh event triggerring for panel, aside the ledger db recording codes.
This is why I said 'need more tests', to check if all the corresponding APIs are handled and are the values in panel always the same with in-game's.

Tech details:

  • Two frequently used functions at global.js: toUTCseconds, toUTChours
  • Replace all Math.hrdInt("floor"... converters with previous ones
  • Add time unit to those ctime vars, such as ...Seconds or ...Hours, fix those function calls with wrong time unit argument.
  • Let more API calls update Resources & Consumables in time (might via delta values only)
  • Improve PlayerManager:
    • Let all side-effect only functions return this instance for chain usage
    • Clearify acceptable time unit of parameters for setResources and other time related functions
    • Let setResources, setConsumables support delta values updating

No feature changed, just unify and refine time handling codes
when timestamp converted to seconds or hours, add time unit to vars name:

 * Two frequently used functions at global.js: toUTCseconds, toUTChours
 * Replace all `Math.hrdInt("floor"...` converters with previous ones
 * Add time unit to those `ctime` vars, such as `...Seconds` or `...Hours`,
 fix those callings with wrong time unit argument.
 * Let more API calls update Resources & Consumables in time
 (might via delta values only)
 * Improve PlayerManager:
   * Let all side-effect only functions return `this` instance for chain usage
   * Clearify acceptable time unit of parameters for `setResources` and other
   time related functions
   * Let `setResources`, `setConsumables` support delta values updating
Copy link
Member

@Javran Javran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allow me to be a bit pedantic here: if you call a function mainly for its effect, that's no longer a side-effect xD. other than that this PR LGTM, might be better if you include some testcases if you find it's tricky to get something correct, not required though.

@@ -366,6 +372,12 @@ String.prototype.hashCode = function() {
return ret;
}
};

Number.prototype.valueBetween = function(lfs, rfs) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might just do function (lfs=-Infinity, rfs=Infinity) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that also works. i just intendedly keep this file with no ES6 syntax for now... 😅

@sinsinpub sinsinpub changed the title Unify time unit & naming at API handling Fix time unit of API handling and refresh panel materials in time Mar 9, 2017
Fix speed docking rsc not tracked
@sinsinpub sinsinpub merged commit e266505 into master Mar 18, 2017
@Javran Javran mentioned this pull request Mar 18, 2017
@sinsinpub sinsinpub deleted the dev/unify-timeunit branch March 19, 2017 04:13
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.

2 participants