-
Notifications
You must be signed in to change notification settings - Fork 25
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
Backport tool charge helpers #391
base: 1.2.x
Are you sure you want to change the base?
Conversation
For test failures: integration tests probably shouldn't be touched and can't really be fixed, not worth it. |
Sounds good 👍 I'd be fine with bumping the version to 1.3.x for this, it's much more than a bug fix. |
Added:
Updated PR description to reflect these additions. edit. Rebasing to keep test / workflow changes separated from actual functionality and documentation. |
Tests for power tools Basic tests for technic_get_charge, technic_set_charge
set_charge/get_charge from other Technic mod Battery box technic_get_charge / technic_set_charge
Click for detailed source code test coverage reportTest coverage report for technic 52.77% in 97/98 files:
Raw test runner output for geeks:
|
I think this should be ready for release. For review, as I imported test fixtures and other test stuff from master and updated workflows there many test related changes. For failing luacheck, failure is completely unrelated to these changes and I really don't want to include fix in this change set. Also don't want to hide that failure, in my opinion better just keep it as is for now and let it report failure. For failing integration tests, should those be disabled, left as is (ignored) or is there easy way to fix them for 1.x branch? I guess @BuckarooBanzay might know the answer. |
Base branch 1.2.x is selected just for sane comparison.
This PR backports following features from (atow) master branch to current 1.2.2 stable release:
technic.get_RE_charge(itemstack)
technic.set_RE_charge(itemstack, charge)
technic.use_RE_charge(itemstack, charge)
true
/false
indicating success.Aliases that are not in Technic Plus Beta but lately added to original Technic, happily had same interface as above stuff:
Tool definition:
<tooldef>.technic_get_charge(stack)
.<tooldef>.technic_set_charge(stack, charge)
.Additions
But why?
Mostly because following PR shows that we're bit too far behind with 1.2.x and 2.x would be a fairly big leap forward, there's version specific code for three versions (and this change would allow dropping one of those):
Notincluded simple things thatcould be easilyhave been backported for compatibility would be at least following tool def functions (just copypasta from master docs):<tooldef>.technic_get_charge(stack)
instead.<tooldef>.technic_set_charge(stack, charge)
instead.This probably should not be merged but instead
in case this gets approved it should make a new release branch, tag and cdb release.
Though in case someone argues this is a bug fix then yeah, 1.2.3 would be fine too.