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

Partial Load feature #177

Closed
wants to merge 2 commits into from

Conversation

kei-gbf
Copy link
Collaborator

@kei-gbf kei-gbf commented May 6, 2019

This patch add dropdown button for partial loading

partial-load

This change makes users manages their save data as Preset.
e.g. Load earth-katana party + Load earth-magna2 weapons.

Save data will become more re-usable.


TODO

  • translate button labels
  • CSS (English locale break button group layout)
  • need review about simulator

This is an experimental feature, yet

The partial loading may break user's save data,
If user had clicked the load button by mistake, but there is no Revert.

Please save a backup, then try at your own risk.

kei-gbf added 2 commits May 7, 2019 03:59
This patch add dropdown button for partial loading

  * Load Djeeta (Player tab)
  * Load Summon
  * Load Chara
  * Load Weapon

This change makes users manages their save data as Preset.
e.g. Load earth-katana party + Load earth-magna2 weapons.

Save data will become more re-usable.

----
This feature is now experimental.

The partial loading may break user's save data,
If user had clicked the load button by mistake, but there is no Revert.

Please save a backup, then try at your own risk.
@kei-gbf
Copy link
Collaborator Author

kei-gbf commented May 13, 2019

screenshot

In progress, how to remove the space.

@kei-gbf
Copy link
Collaborator Author

kei-gbf commented May 26, 2019

I will withdraw this PR. The concept was duplicated with #201 and it's better approach.
After I implemented this feature, I noticed several issues.

  1. Server may store an uncompleted data,
    if users made the partial data then save to server.
    I assumed users save it to browser. at least everyone have to make the initial data.
    Add preset URL parameter #201 approach, server admin can provide the initial data and users can share use.
    This approach can't provide the "share"

  2. maintenance cost
    no need to provide same concept by two ways. when Add preset URL parameter #201 was implemented, this will be an unnecessary feature.

I was going to tell this in #201, add UI code in current code base increase the legacy code.
Motocal use react bootstrap old version, when we need update the version, the cost is increased.
in #201 approach, I assume external program/UI can provide the preset.
e.g 3rd party tool, they can start new repo with new libraries.

  1. "simulator" section in the state (save data)
    I had not use damage simulator,
    I am not sure if this patch had side effect to the simulator feature.
    If this PR will be merged, need much tests about simulator.

There are no E2E tests now, test manually is hard to do.

I will still keep this PR until #201 close. (for discussion if someone had ideas)
There is also an advantage, users can make custom initial data.
instead of server provide preset.

I will show #201 contains the most of this PR aimed then this PR will be unnecessary.


このPRは撤回予定です。理由は主に3つ、実装してみてわかった事。(要約)

  1. サーバに不完全なデータが保存される可能性ある。2. 保守性の問題。
  2. ダメージシミュレータ関連に影響がありそうだけどテストが困難。

議論の為に #201 が閉じるまで残しますが、
このPRで提供するはずだった機能は、#201 で(別の形で)実装する予定です。

@kei-gbf
Copy link
Collaborator Author

kei-gbf commented May 26, 2019

Other comparison #177 and #201
Think how many steps users takes

  • Partial Load feature #177 case User select
    1. Djeeta profile//select load profile/load
    2. Magna summons/select load summons/load
    3. Magna2 weapons/select load weapons/load
    4. Katana chara/select load characters/load
  • Add preset URL parameter #201 case (future scenario)
    Although Add preset URL parameter #201 does not provide the UI directly,
    but make it possible 3rd party tool can provide like this:
    Users can click/select Element/Magna summons/Magna2 weapons/Katana chara then click Load

EDIT: fix PR number 171 -> 177

@kei-gbf
Copy link
Collaborator Author

kei-gbf commented Jun 1, 2019

I will restart this topic in #201 soon, close here.

@kei-gbf kei-gbf closed this Jun 1, 2019
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.

1 participant