Improve support for encoding alternative number formats into stats when constructed
Closed this issue · 6 comments
Assigning to a fixed point stat (one with a val shift and encoded as a fixed point number) will be easier with a constructor that takes a float arg which then can be encoded properly into the fixed point int representation. Fixed point bounds are defined as ints and thus need to be encoded into fixed point via shift when assigned -- I think it makes more sense to pass in a (float) casted arg so the correct overloaded constructor gets called. When implementing, it makes sense to add an invariant that the float arg has no fractional part, since that is not the use-case of the constructor.
Math for stats must not use floats, as there may be rounding errors and it makes more sense to decode them as a float for display.
Additionally, Stat#toString outputs the int representation of the stat. I think it may be correct to assume that valshift indicates that the value is a fixed point number, and thus treat it as a new encoding. It's also safe to make the assertion that only stats with encoding=0 support valshift
Looking through It's a poor readability thing because of operator priority. I'm going to improve it.#toString more, I think it need to be modified. return id + "(" + entry + ")" + "=" + (entry.Save_Param_Bits == 0 ? Integer.toString(val) : val + ":" + param); the ternary ? : operator doesn't make sense.
Looking into this issue, I'd also like to address the issue with also encoding long stats (really a 32u). I've been thinking of this for a while now, so I'm experimenting with an improved stats implementation to address the deficiencies in this iteration.
I've been doing some experimenting with alternative implementations for stats and how the API is designed. Still more work to be done, but so far it looks good. If the current iteration I'm working on doesn't make it in, then I'll make another issue with the (hopefully) final iteration. I'm hoping my current work solves this issue, fixes some mutability issues, and significantly improves memory usage by stat lists -- it's been long overdue knowing how more of this fits together.
FYI, this issue popped up while assigning the health to monsters, so the other work I'm doing is regarding how monsters attributes are handled. Giving a monster an attributes works, however what I'm doing should be far more efficient for a data structure that will have maybe thousands of instances of.
API is flushed out enough to start playing with implementation details. I'm extremely happy with the changes thus far. I'd like to come back at some point and implement android-esque annotations (especially if IntelliJ supports them) due to how programmer-error prone some parts may be, however I've doing my best to limit this complexity to within com.riiablo.attributes. If all goes well, I can add documentation and maybe explain how it works and why it's far improved from the old implementation.
I looked into implementing com.android.support support annotations, but there is no support for ShortDef and IntDef requires an int or long. What a shame (stat ids are short). I want to see if there is an alternative library (outside of the android ecosystem, specifically for validating primitives). I also looked into annotation processors in general, and there is a high likelihood of implementing some in the future to assist with code generation.
The stats revision is completed and test cases pass for a reasonable number of items. Rolling module into production so this issue can be closed.
I missed some features which still need to be added. Good news is that this new iteration is going very smoothly.
Issue has been resolved. More improvements will come as necessary.