mod-autobalance icon indicating copy to clipboard operation
mod-autobalance copied to clipboard

Divide et impera

Open kewinrausch opened this issue 1 year ago • 2 comments

Changes Proposed:

  • A single file implementation split into multiple files. Each class has its own file, which allows to split complexity of the module into multiple black boxes that can be better understood and extended in the future.

Issues Addressed:

  • No issues addressed

SOURCE:

Tests Performed:

  • Compiles with no error and no deprecation warning
  • Tested into private server into a dungeon instances; seems to work as old version since no changes has been performed on the logic of the module and balancing.

How to Test the Changes:

  1. Pull the changes
  2. Compile the server
  3. Enter and complete and instance; i did not touch internal module mechanism except for splitting the classes into multiple files and some decorations.

Take you time to confirm that the module work as intended. I'm personally performing additional tests to be sure the balancing is not affected. I expect no problem since i just moved code around files and did not touch the code inside the classes and procedures.

kewinrausch avatar Sep 21 '24 12:09 kewinrausch

it's a big PR, not sure if I have the time to review it soon :(

Helias avatar Sep 21 '24 19:09 Helias

Do not worry, i have no rush or bad feelings about it. Take all the time you need. I will keep playing using this and test out if something is broken on my side.

kewinrausch avatar Sep 22 '24 09:09 kewinrausch

I think we should merge this asap, maybe we can "versioning" this module and add to the readme the commit/link to the version before this as a stable version, and this will stay in "beta" until we ensured that everything is working

unless there's someone that wants to test it extensively right now, but I'm afraid that the more time we wait the worse it will be to resolve the conflicts with other incoming PRs

Yehonal avatar Sep 26 '24 13:09 Yehonal

@kewinrausch you could add a note in the README.md about this PR, saying that from this PR the module could be "unstable" and we'll merge the PR ;-)

I appreciate the big effort here, this note it's only to keep track of this

Helias avatar Sep 26 '24 14:09 Helias

I did as you requested.

Wouldn't it be better to create also a tag or a branch pointing to the stable version right now? Once enough time has passed and enough test have been performed the branch can be removed to maintain only the master one.

kewinrausch avatar Sep 26 '24 16:09 kewinrausch

done ✅

Helias avatar Sep 26 '24 18:09 Helias