PoESkillTree icon indicating copy to clipboard operation
PoESkillTree copied to clipboard

Computation rewriting

Open brather1ng opened this issue 7 years ago • 12 comments

Our current calculation code is heavily outdated and hard to maintain, so I plan to completely rewrite what Compute.cs does from scratch and at least do major refactoring to ItemDB.cs and other calculation related stuff.

Not sure when I'll get to start with it, but might as well open an issue for discussion now.

Some thoughts:

  • Completely separate computation/mechanics from everything data related (like mod parsing). So most maintaining would only have to happen on the data side of things.
  • Make the data related things easy to understand and maintain. There should not be much logic in there, maybe a few things for conditions that only appear once. Could potentially be done completely as xml/json files, though I think that's too much work for not much gain.
  • Calculations could be represented in some kind of object tree (with the resulting stat as root, e.g. total maximum life) that supports on-the-fly updating, i.e. skill a node that gives one stat and it cascades up to the root. It would allow previewing changes to calculated stats when hovering over nodes.
  • Calculations would optimally be fast enough to allow usage of calculated stats in the tree generator. This should generally be supported by the architecture, i.e. no dependencies on skill tree related things and no static state (atm we have lots both).
  • In addition to skill tree and items as main inputs, it should allow enabling buffs/debuffs, e.g. auras, flask effects and active charges.

Path of Building is obviously a big source of both motivation and inspiration, as it makes our current calculation stuff completely obsolete. (as a side note, from what I've seen from its code, it doesn't look to good on the maintainability side either)


With the parsing now having an open Pull Request (#500), here is what I'd say makes sense as milestones:

  1. [x] Parsing (PR: #500)
  2. [x] Computation itself (all the calculation things), with very minor UI integration (Issue: #501, part 1 PR: #509, part 2 PR: #512, part 3 PR: #515, part 4 PR: #517)
  3. [x] Jewel support (computation PR: #531, equipment UI PR: #537, skill tree UI PR: #542)
  4. [ ] Full UI integration (Issue: #545)
  5. [ ] Tree Generation integration

Full UI integration doesn't necessarily need to be done in one part, some parts might make sense before jewel support and some might be less important than Tree Generation integration. Jewel support is not directly related and still the easiest thing for someone else to pick up from these, but I do think it is required for the program to be called a full build calculator.

I'll open a new issue for discussion and status updates about the next milestone once I'm starting working on it. I'll try to divide that into multiple PRs and comment about the current state more often.

brather1ng avatar Apr 22 '17 16:04 brather1ng

Fully agreed. Aside from the obvious "just go to a webpage instead of downloading yet another tool", their damage calculations currently sweep the offline planner out the door. A rewrite that focuses both on maintainability and calculation efficiency (plus integration with the GA) could put us back into competition.

I'm still quite unsure about the effort of keeping things up to date wrt new uniques and such, but I guess that's what open source is for.

I'll have some spare time to devote in the coming weeks, so if you think I can help you in any way, do let me know!

MLanghof avatar Apr 23 '17 00:04 MLanghof

I'm still quite unsure about the effort of keeping things up to date wrt new uniques and such, but I guess that's what open source is for.

My goal is that adding a mod for a new unique would only be one line of code in the data part that handles conditions, values, affected stats and the likes (which obviously only works if the mod is not a completely new mechanic). That would be not much effort and also easy for others to contribute if the syntax/functions used are easy to understand.

I'll have some spare time to devote in the coming weeks, so if you think I can help you in any way, do let me know!

I didn't do anything more on the topic yet than writing this issue and collecting points on my todo list over time, but I'm sure another helping hand would be great!

brather1ng avatar Apr 23 '17 10:04 brather1ng

Took a while, but I'm done with everything I wanted to do before this and I should have at least some time now (and more time in a few weeks) to spend on this. So if you still got time and want to help me, that'd be great!

brather1ng avatar Jun 15 '17 16:06 brather1ng

I won't have a terrible amount of free time in the near future, but I'm totally down for working on this. What do you suggest for coordination? Github tickets in your repo (so we don't spam this one), meeting on Skype again or something entirely different?

MLanghof avatar Jun 15 '17 18:06 MLanghof

I suggest GitHub issues here for the bigger things, so others can also easily join the discussion, and Skype (or Discord) for coordination

brather1ng avatar Jun 15 '17 19:06 brather1ng

We could create a poeskilltree discord. It would give everyone a way to talk with each other. We can also create support channels and what not

EmmittJ avatar Jun 15 '17 19:06 EmmittJ

Sounds like an excellent idea!

MLanghof avatar Jun 15 '17 19:06 MLanghof

I'm also totally up for that!

And welcome as Collaborator, @MLanghof!

brather1ng avatar Jun 15 '17 19:06 brather1ng

Thanks! (Also thanks @EmmittJ :D)

MLanghof avatar Jun 15 '17 19:06 MLanghof

:) no problem

EmmittJ avatar Jun 15 '17 19:06 EmmittJ

I have created a Discord server. I have no idea about Discord server administration, but I guess I can just invite you, give you enough permissions and we can figure it out.

brather1ng avatar Jun 15 '17 20:06 brather1ng

I've edited the issue with a roadmap on what I'm planning to do so that there's an overview for my plans and the current state somewhere:

With the parsing now having an open Pull Request (#500), here is what I'd say makes sense as milestones:

  1. [ ] Parsing (PR: #500)
  2. [ ] Computation itself (all the calculation things), with very minor UI integration
  3. [ ] Jewel support
  4. [ ] Full UI integration
  5. [ ] Tree Generation integration

Full UI integration doesn't necessarily need to be done in one part, some parts might make sense before jewel support and some might be less important than Tree Generation integration. Jewel support is not directly related and still the easiest thing for someone else to pick up from these, but I do think it is required for the program to be called a full build calculator.

I'll open a new issue for discussion and status updates about the next milestone once I'm starting working on it. I'll try to divide that into multiple PRs and comment about the current state more often.

brather1ng avatar Jan 01 '18 23:01 brather1ng