CombustibleToast / StratagemHeroOnline

36 stars 23 forks source link

Feature: drill mode #35

Open Baksztag opened 2 months ago

Baksztag commented 2 months ago

Resolves https://github.com/CombustibleToast/StratagemHeroOnline/issues/34

Baksztag commented 2 months ago

@CombustibleToast There is a lot of duplication, especially in drill.js, so it might be a good time for some refactoring. For now I just copied over index.html and script.js and worked on the copies to avoid breaking the main functionality and to leave the decision on how to approach the duplication to the maintainers. Personally, I'd suggest moving core functions and game state to one script file (e.g. core.js) and loading it together with page-specific script which uses necessary functions only. Loading the scripts as JS modules might help with clear code organisation. Also unit tests would be great to avoid breaking the game during refactoring.

choidavid4 commented 2 months ago

I was testing this out with some stratagems, and it gets to a point where it stops scrolling them.

image image

Fixed it changing pickNextStratagem() function inside drill.js to the following

function pickNextStratagem() {
  let nextStratagemIndex = Math.floor(Math.random() * seletecedStratagems.length);
  return seletecedStratagems[nextStratagemIndex];
}
Baksztag commented 2 months ago

I'll try to check it today. @choidavid4 what was the stratagem config that you started with?

choidavid4 commented 2 months ago

These are the Stratagems that I selected

  1. Reinforce
  2. Resupply
  3. Hellbomb
  4. Orbital Laser
  5. Orbital Railcannon Strike
  6. Eagle 500kg bomb
  7. Eagle rearm
  8. Shield Generator Pack
Baksztag commented 2 months ago

@choidavid4 thanks! I found the culprit – Shield Generator Pack was duplicated in the data. You've hit an edge case that wouldn't occur if you had anything else as selected as the last stratagem. I deleted the duplicate so now it works as intended. You must either clear your localStorage to make it work or select other stratagem set.

Baksztag commented 2 months ago

Regarding the fix

Fixed it changing pickNextStratagem() function inside drill.js to the following

It changes the behavior a bit as it introduces random ordering. We could add it as another config option if you'd like.

choidavid4 commented 2 months ago

I think random ordering is the way to go. It helps training because you don't know what stratagem comes next.

I'd love to have the choice to enable it in a config.