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

Computation rewriting #463

Open
3 of 5 tasks
brather1ng opened this issue Apr 22, 2017 · 12 comments
Open
3 of 5 tasks

Computation rewriting #463

brather1ng opened this issue Apr 22, 2017 · 12 comments

Comments

@brather1ng
Copy link
Member

brather1ng commented Apr 22, 2017

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. Parsing (PR: Computation Rewriting Part 1: Data and Parsing #500)
  2. Computation itself (all the calculation things), with very minor UI integration (Issue: Computation Rewriting Part 2: Calculations #501, part 1 PR: Calculation Rewriting Part 2.1: Core Data Structure #509, part 2 PR: Calculation Rewriting Part 2.2: Mechanics and Builder Implementation #512, part 3 PR: Calculation Rewriting Part 2.3: Skill and Item parsing #515, part 4 PR: Calculation Rewriting Part 2.4: Basic UI #517)
  3. Jewel support (computation PR: Jewel Support Part 1: Computation #531, equipment UI PR: Jewel Support Part 2: Equipment UI #537, skill tree UI PR: Jewel Support Part 3: Skill Tree UI #542)
  4. Full UI integration (Issue: Integration of Computation into UI for 3.0 alpha #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.

@MLanghof
Copy link
Collaborator

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!

@brather1ng
Copy link
Member Author

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
Copy link
Member Author

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!

@MLanghof
Copy link
Collaborator

MLanghof commented Jun 15, 2017

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?

@brather1ng
Copy link
Member Author

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

@EmmittJ
Copy link
Member

EmmittJ commented Jun 15, 2017 via email

@MLanghof
Copy link
Collaborator

Sounds like an excellent idea!

@brather1ng
Copy link
Member Author

I'm also totally up for that!

And welcome as Collaborator, @MLanghof!

@MLanghof
Copy link
Collaborator

Thanks! (Also thanks @EmmittJ :D)

@EmmittJ
Copy link
Member

EmmittJ commented Jun 15, 2017 via email

@brather1ng
Copy link
Member Author

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
Copy link
Member Author

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: Computation Rewriting Part 1: Data and Parsing #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants