-
Notifications
You must be signed in to change notification settings - Fork 76
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
Add preset URL parameter #201
base: master
Are you sure you want to change the base?
Conversation
This patch implements two another ways to load JSON data. * preset=HASH loads base64 encoded json data Data body is read from location.hash (javascript) Server access log will not store the long data. * preset=name loads ./json/name.json file "name" is checked \w\-\_ letters only Internal changes: * loadDatachar (decoded json text) data load function, the original code is moved here. * getDatacharByFragment (preset=HASH) * getDatacharByURL (preset=name) load raw json text, no base64 encode. * getDatacharById (id=num)
?load=HASHSample URL: server url is when the
NOTICE: "HASH" is a fixed token for loading from URL fragment. ※ URLフラグメントはクライアント側で処理されるので、 EDITED: rename parameter key How to generate the parameterCurrently, I do not provide this.
パラメータは、JSON テキストを Base64 エンコードしたものです。 注意1: 元カレで出力したJSONには複数のデータがあるので、必要な部分を抽出してください。 一応、テストの為に手順を書きましたが、 use caseFor dynamic parameter generation. スクリプト等で動的にパラメータを生成する為に使います。 |
I think this long of a hash can cause issues with most browsers. Base64 also depends on characters such as '+', '=', '/' which is not url safe and need to be re-encoded in order to be url safe. I recommend switching to Base62 aka what Google uses. For size concerns I have two recommendations:
Length: 903 Base62 Converted Hash:
Length:1214 Same data can be represented as following encoding:
So data becomes this:
Length: 142
With Base62 Encoding:
Length: 192
Original size: 142 bytes Result size: 97 bytes With Base62:
Length: 178 |
thx information. it's good point, base64 is not url safe. I did not think that.
The Base 62 encoding is used for URL short service ? (need to be in http request path) Known issue is: RFC3986 Uniform Resource Identifier (URI): Generic Syntax
|
Base62 is url safe. Google uses it in Youtube and Gmail in order to pass item id. You can check following website to try it: |
👍
Some browser has issue on copy such too long text on location field.
base62 is used in
|
I do not plan the fixed column format, that lose flexibility. A compression is a good optimization point. |
another idea: (I am thinking rename "preset" -> "load")
|
Yep that makes more sense if you want to support files.
I also wouldn't support encoding based on string replacement. I would assume if that choice is made, there would be middleware class/function to convert our json save data to that encoding as well as convert that encoded data to our java. So updating it becomes non-issue as it will be "auto updated" once someone edits our save.json |
middle ware concept sound nice. is it like python's WSGI middle ware? I may assume the hash is use only internal. I have related topic #177 the goal will be
I do no plan user-interface, this just provide the lower layer. I'll continue this next next week. |
This patch provides flexible ways to load JSON data. Load from JSON files on server and URL fragment. Loaded data will be applied "REPLACE" or "MERGE" strategy, then compose new state to load in Motocal. The use cases are third party program generate parameter, Reviewers checks the save data compatibility. see MotocalDevelopers#201 for more details.
The entry point is componentDidMount: async function () {
/**
* URL parameter handling, now implement preset loading feature.
*
* 1. get_url_parameter url_param.js
* 2. represent_load_command preset_param.js
* 3. promise_download preset_download.js
* 4. apply_patch_processor preset_patch.js
* 5. make_state preset_state.js
*
* @see PR #201 for details. (@kei-gbf)
*/ Checklist for review
I add many files, they could be single file but mixing es2015 style, import/require because jest did not transform, |
and code clean up.
Encodingconst DESERIALIZE_METHOD = JSON.parse;
const DECODE_METHOD = Base64.decode; Now preset_download.js file has the configuration, I choose base64 for now, two reasons:
However, browser implementation may not obey the spec. TODO: need test base62 library at least |
Can you add @babel/polyfill dependencies to package.json and webpack entry point. Without it async commands sometimes don't play well with babel-loader. |
Tthx for the information, I will try it later. There are two ways to run the code:
TODO: research those libraries
|
What type of test data you need? Save data? |
Yes, it's JSON file under ./json/preset in this PR repo.
|
|
in the test case, I assume it's actual data that Motocal generated. Second case, anyways ok current status of this PR, I already implemented main features.
I will give up esdoc, that pending task |
thx, personally I had not use motocal save data so deep. seem that can be use for stress-test (loading big data) for preset use (two use cases)
|
I don't think most site admins will create complicated save files like this. These are mostly end user save files. |
you can see some examples here. these are rather old. and uses different variable names to most this repo for a good amount of stuff. I have not dive deep into how motocal data saving. |
thx, in test case any file can be used, it test outdated, illegal format etc
currently category names are used as directory "profile", "chara", "summon", "armlist" except "preset"
@h-yasha
is it same issue #226 ? re-produce the error
the loading part is re-using current data save/load, via |
This patch solve es2015 style module `import` that used in unit-test for this preset modules. - Install babel-plugin-transform-es2015-modules-commonjs - package-lock.json updated - package.json: - Add config for Jest - .babelrc: - Add config enable plugin for NODE_ENV=test
This parameter support to load json file under `./json/test/*.js` Internal changes: - Separate variable category and directory - content.js - preset_param.js - preset_download.js Minor changes: - Fixed: preset.test.js was not tested before. - now the test passed. - Fixed: typo in document, zip in zip_longest function.
Those .json files are temporary test file during this PR. When it's ready to merge, I may tidy or remove them.
This data just test file loading, The set rank 20 is just changes for data change check. - `?test=rank20` for test this change.
NOTE: now unit test failed in TravisCI, because Jest config changed. |
You can replicate it by loading 20 weapons. Issue is it only renders first 16 weapons before you go to weapon tab, go to another tab and go back |
I found the unit tests problem was difference of Jasmin(CodePen.IO) and Jest(Motocal) |
thx info. I did not try load those files yet, added test= parameter
|
My advice on that would be keep them as a test but add CRC hash of the to end of the it. That way you can make sure all files are unique. |
This was happen by the difference between Jasmin(CodePen) and Jest(Motocal) It's repored in official jest issue Jest fixed the re-use of `this` context as bug. (it's implicitly context binding to use `this` in test code) so Jasmin's test code does will not work in Jest. This patch is required in MotocalDevelopers#201
👍 CRC for freeze the file contents. (for test use case) Other info I want them to include is Date or PR number |
This was happen by the difference between Jasmin(CodePen) and Jest(Motocal) It's repored in official jest issue Jest fixed the re-use of `this` context as bug. (it's implicitly context binding to use `this` in test code) so Jasmin's test code does will not work in Jest. This patch is required in #201
missing in MotocalDevelopers#266 I noticed after merged. this fix is required in MotocalDevelopers#201
missing in MotocalDevelopers#266 I noticed after merged. this fix is required in MotocalDevelopers#201
@kei-gbf |
features are almost done. one of motivate of this PR was for help the review tasks, though. This patch was implemented as ./preset module, less hard code patch as possible, I will just note my view point and use scenario for share info: Input -> Calc Logic ->Output report (result.js) currently Motocal contains all in one. (strong module dependency)
This was for Input/import, the same to Output/export layer
I do not assume who make those 3rd party implementation, It's not require full set of web app. for example second use scenario is for improve Review/Test costs If PR author of update patch provides the JSON file of new chara/weapon |
This patch provides another way to load JSON data.
JSONデータを読み込む為の、presetパラメータの実装。
サーバ上に設置した JSON ファイルから読む方法と、
URLフラグメントにエンコードしたJSONデータを渡す方法を提供します。
preset=HASHload=HASH
loads base64 encoded json dataData body is read from location.hash (javascript)
Server access log will not store the long data.
preset=name
loads ./json/preset/name.json file"name" is checked \w-_ letters only
EDIT: info updated ./json/ has now sub directory -> ./json/preset
Use cases
I assume a scenario: server owner can provide top-page for motocal
the top page has change-log, link to open the preset for the template settings.
sometime we add new parameter to the JSON save data,
reviewers must test the old save data is safe to load.
we can share the test save data in repo then test load by preset.
用途
例えば、トップページを用意して、ユーザが選択する形式を想定。
特定の敵相手のテンプレート編成等を提供できると便利。
過去のセーブデータを開けるかどうかのテストが必要になります。
テスト用データをレポジトリで共有できると、テストがやりやすくなる。
Checklist
?load=HASH
(I will explain this in next post)maybe issue for Edge, Safari, they allow only 2k/4k but do not care this here.
for chrome and debug only use. most browser allow more long data.
?preset=name
?preset=test
(minimum sample data, just edit rank 20)?preset=empty
... will show error now. (not much test error cases)?preset=no-exists-file
... test something if file not existsinitState["dataName"] = "serverData";
in loadDatachar() need to change dataName?Parameter (stage: need test)
I am thinking rename "preset" -> "load"Implemented preset and load, preset is an alias.
?preset=preset-name
... alias for load=FILE&file=?preset#preset-name
...XXX: not this. server log can't record "preset-name"?load=FILE&file=preset-name
?load=PATCH#{encoded JSON}
... merge?load=HASH#{encoded JSON}
.... replace?load=PART%profile=djeeta-buff&chara=earth-katana&armlist=earth-magna2&summon=earth-magna
?load=FILE,PATCH&file=preset-name#{encoded JSON}
?test=rank20
name
parameter for data nameUpdate (2019/06/16): Add test= parameter
実装予定のパラメータ。(名称は preset から loadに変更予定)2019/05/18 現在試せるのは、?preset=test
と?preset=HASH#...
のみHELP WANTED
テスト用のデータ作成等。
TODO
esdoc: namespaceesdoc: fix function signature mismatchThis PR will close #177 similar concept feature.