search

Retera / WarsmashModEngine

An emulation engine to improve Warcraft III modding
GNU Affero General Public License v3.0
231 stars 42 forks source link

Adding Human and NightElf Build #5

Open MfromAzeroth opened 2 years ago

MfromAzeroth commented 2 years ago

Implementation of Human and NightElf construction with power build, repair, wisp consumption and costs

Retera commented 2 years ago

Hi! I'm part of the way through reviewing this and I like the direction it's going. But I noticed that this pull request is targeting main instead of experimental, which means that later if we merge experimental into main then there will be conflicts with the contents of this branch. In particular, we are now in a weird state where this pull request contains a better human powerbuild solution, but also a hit point regeneration solution that if I'm not mistaken doesn't honor blight/night, whereas the "experimental" branch powerbuild is worse (does not appropriately charge gold/lumber) but has a hit point regeneration system that does honor blight/night. So, I have been a tad slow to merge this pull request because (although it's kind of on me for branching off with the experimental branch) I was avoiding dealing with the inevitable merge conflicts.

Do you have a preference on this? Are you planning to wait until I resolve this conflict somewhere in my workflow, or are you interested in resolving it on your end and updating the pull request to target the "experimental" branch?

MfromAzeroth commented 2 years ago

Sorry for taking so long to reply. Life is busy right now. Thanks for the feedback. I’ve changed the pull request to target experimental. I’d be happy to resolve these issues, but it might take a few weeks for me to get around to everything, so don’t let me delay you, if you want to make quick progress or something. Maybe let me know if you fix something in case I take too long. Otherwise, I’ll reply to the individual comments once I get around to the concrete things you ask me to do there.