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

First pass. Go easy on me. #224

Open
wants to merge 9 commits into
base: Development
Choose a base branch
from
Open

First pass. Go easy on me. #224

wants to merge 9 commits into from

Conversation

Doc-Saintly
Copy link

Still lots of work to do, but to add more modules it's basically just:

  • Add a view
  • Add a factory method
  • Add it to the resolver

Copy link

@MikeJeffers MikeJeffers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a first pass for now, statically reading, I'll pull the code soon (tm)
For now just want to hear your thoughts on some of your decisions and if parts could get a little tighter
The big thing that sticks out is that new method on the entityframework and the way its used typechecking a raw object return.

src/Perpetuum/Modules/SlotFlags.cs Outdated Show resolved Hide resolved
src/Perpetuum/EntityFramework/EntityFactory.cs Outdated Show resolved Hide resolved
src/Perpetuum/EntityFramework/EntityFactory.cs Outdated Show resolved Hide resolved
Perpetuum.DataDumper/Views/ModuleERPDataView.cs Outdated Show resolved Hide resolved
Perpetuum.DataDumper/Factory/DataDumper.Resolver.cs Outdated Show resolved Hide resolved
Perpetuum.DataDumper/Factory/DataDumper.ModuleERP.cs Outdated Show resolved Hide resolved
Perpetuum.DataDumper/Factory/DataDumper.AmmoWeapon.cs Outdated Show resolved Hide resolved
Copy link

@MikeJeffers MikeJeffers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally pulled the code, some small fixes are needed.

Perpetuum.DataDumper/DataDumper.cs Outdated Show resolved Hide resolved
Perpetuum.DataDumper/DataDumper.cs Outdated Show resolved Hide resolved
docsaintly and others added 7 commits October 11, 2020 17:21
Changed localization parser to use GenXy deserializer
Added driller module
Removed changes to entity factory
Fixed SlotFlags comment
Removed some unnecessary comments
Added option to open exported file
Added more module categories
Copy link

@MikeJeffers MikeJeffers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note on all the hardcoded coefficients on the data for display are stored in the DB.
And it looks like the client might actually apply the transforms as the only server side usage appears here: https://github.com/OpenPerpetuum/PerpetuumServer/blob/Development/src/Perpetuum.RequestHandlers/GetAggregateFields.cs#L24-L28
Might save time on transforming other data/fields later.


// Same as input.GetPropertyModifier(AggregateField.locking_range).Value * 10;
LockingRange = input.MaxTargetingRange * 10;
LockingTime = input.GetPropertyModifier(AggregateField.locking_time).Value / 1000;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AggregateField should have things like display units or display multiplier, which are transforms on the number for the purposes of rendering in a view for users.

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

Successfully merging this pull request may close these issues.

3 participants