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

UPS follow up fixes #9664

Merged
merged 3 commits into from
Oct 25, 2014
Merged

UPS follow up fixes #9664

merged 3 commits into from
Oct 25, 2014

Conversation

BevapDin
Copy link
Contributor

Fixes the issues from that comment over there #9652 (comment)

Minor complaint: the UPS can be activated but that doesn't do anything now. Not a dealbreaker but should be fixed before stable. ;-)

Seems to be a general problem, it's the same for the both forges. They have no iuse function, so the game does nothing when invoked. I added a message for that case.

Not sure if this PR causes it, but wielded powered-items with a UPS conversion installed don't charge. Put it back in main inventory and it charges as expected.

That seems to have been overlooked in eff239b, it needs to check against maximum_charges like it does for items in the inventory, fixed.

Minor quality-of-life issue: tanksuits don't indicate their on/off status at the moment

That's another bug. I've only seen them yellow in the inventory window, that was enough for me. But I added another item suffix for those items that are active, but have no separate active item type. Usually the active item type has the id "foo_on", so it checks whether the item id has the suffix "_on" and if not adds "(active)" to the item name.

@KA101 KA101 self-assigned this Oct 24, 2014
if( active && !is_food() && ( type->id.length() < 3 || type->id.compare( type->id.length() - 3, 3, "_on" ) != 0 ) ) {
// Usually the items whose ids end in "_on" have the "active" or "on" string already contained
// in their name, also food is active while it rots.
ret << _( " (active)" );
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with corpses. Test targets for the shotgun PR were (active) after I'd downed 'em. This was a bad idea on my part, and I'll remove/fix it as part of the merge. Sorry to waste your time, BevapDin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, I'll add a the tag in the inventory ui, and only for worn items.

Copy link
Contributor

Choose a reason for hiding this comment

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

I added a !is_corpse() check. Seemed to achieve the goal.

(Reason tanksuits benefit from the (active) is that when they're highlighted in the inventory--which they tend to be, since you can't wear anything under them--the highlight cancels the yellow text. So you'd need to have a helmet or frame, then flip over to that list and move the highlight down to check.)

@KA101 KA101 merged commit 1d3b8eb into CleverRaven:master Oct 25, 2014
@BevapDin BevapDin deleted the ops-the-followup branch October 25, 2014 11:40
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.

2 participants