Skip to content

/Cataclysm-DDA

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

Only charged electronic devices will show time and work as an alarm clocks #28485

Merged
merged 5 commits into from Mar 8, 2019
+8 −5

Conversation

Reviewers

@kevingranade kevingranade

@AMurkin AMurkin

Assignees
No one assigned
Labels
<Bugfix> Items / Item Actions / Item Qualities
Projects
None yet
Milestone
No milestone
4 participants
@Night-Pryanik @kevingranade @rangilin @AMurkin
@Night-Pryanik
Copy link
Member

commented Mar 4, 2019

Summary

SUMMARY: Bugfixes "Fixed electronic devices with 0 charge still showing time."

Purpose of change

Fix issue mentioned here.

Describe the solution

Check if an item has an appropriate flag (WATCH) and has max_charges field (= it accepts batteries, like game watch or cellphone). If if does, check if the item has at least one charge. If either of the checks fail, bail out. Otherwise, item can show time.

If an item has the flag, but doesn't accept batteries (like gold watch), it can show time.

@Night-Pryanik Night-Pryanik added Items / Item Actions / Item Qualities <Bugfix> labels Mar 4, 2019

@kevingranade

This comment has been minimized.

Sign in to view
Copy link
Member

commented Mar 4, 2019

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/cellphones-show-the-time-with-0-charge/19041/4
@rangilin

This comment has been minimized.

Sign in to view
Copy link

commented Mar 4, 2019

I could be wrong as I am not familiar with the code base, but I feel like this change can cause unintended behaviors.

For example:

  • Camera Pro will lose CAMERA_PRO, ALWAYS_TWOHAND flags when there is no battery left.
  • Chainsaw will lose NONCONDUCTIVE flag when there is no gasoline left.
@Night-Pryanik

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 4, 2019

Yeah, it seems you are right. Will work on it.

@Night-Pryanik Night-Pryanik changed the title Only charged electronic devices will show time [WIP] Only charged electronic devices will show time Mar 4, 2019

@Night-Pryanik Night-Pryanik changed the title [WIP] Only charged electronic devices will show time Only charged electronic devices will show time and work as an alarm clocks Mar 4, 2019

@kevingranade kevingranade changed the base branch from development to master Mar 5, 2019

@kevingranade kevingranade changed the base branch from master to development Mar 5, 2019

@kevingranade

This comment has been minimized.

Sign in to view
Copy link
Member

commented Mar 5, 2019

I'm not sure what's happening, but this is causing crashes:

The program has crashed.
See the log file for a stack trace.
CRASH LOG FILE: ./config/crash.log
VERSION: 0.C-37597-g8c399e4502
TYPE: Signal
MESSAGE: SIGABRT: Abnormal termination
STACK TRACE:

        ./cataclysm-tiles(_Z21debug_write_backtraceRSo+0x43) [0x555eb69053b9]
        ./cataclysm-tiles(+0x3d190b) [0x555eb68f690b]
        ./cataclysm-tiles(+0x3d1b3b) [0x555eb68f6b3b]
        /usr/lib/libc.so.6(+0x37e00) [0x7f5daf1a8e00]
        /usr/lib/libc.so.6(gsignal+0x10f) [0x7f5daf1a8d7f]
        /usr/lib/libc.so.6(abort+0x125) [0x7f5daf193672]
        /usr/lib/libc.so.6(+0x22548) [0x7f5daf193548]
        /usr/lib/libc.so.6(+0x30396) [0x7f5daf1a1396]
        ./cataclysm-tiles(_ZNK4cata8optionalI10islot_toolE3getEv+0x29) [0x555eb6857953]
        ./cataclysm-tiles(+0x809ed8) [0x555eb6d2eed8]
        ./cataclysm-tiles(+0x95b154) [0x555eb6e80154]
        ./cataclysm-tiles(_ZNSt17_Function_handlerIF13VisitResponseP4itemESt8functionIFS0_PKS1_EEE9_M_invokeERKSt9_Any_dataOS2_+0x33) [0x555eb6e83295]
        ./cataclysm-tiles(_ZNKSt8functionIF13VisitResponseP4itemEEclES2_+0x1d) [0x555eb6e86db5]
        ./cataclysm-tiles(+0x95ac55) [0x555eb6e7fc55]
        ./cataclysm-tiles(_ZN9visitableI9CharacterE11visit_itemsERKSt8functionIF13VisitResponseP4itemS5_EE+0x52) [0x555eb6e7fe74]
        ./cataclysm-tiles(_ZN9visitableI9CharacterE11visit_itemsERKSt8functionIF13VisitResponseP4itemEE+0x40) [0x555eb6e838ca]
        ./cataclysm-tiles(_ZNK9visitableI9CharacterE11visit_itemsERKSt8functionIF13VisitResponsePK4itemEE+0x43) [0x555eb6e85495]
        ./cataclysm-tiles(_ZNK9visitableI9CharacterE13has_item_withERKSt8functionIFbRK4itemEE+0x40) [0x555eb6e855be]
        ./cataclysm-tiles(_ZNK6player18has_item_with_flagERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEb+0x4e) [0x555eb6d2c4ca]
        ./cataclysm-tiles(_ZNK6player9has_watchEv+0x56) [0x555eb6d31490]

        Attempting to repeat stack trace using debug symbols...
        backtrace: popen(nm) failed
        backtrace: popen(nm) failed
        backtrace: popen(nm) failed
        backtrace: popen(nm) failed
        backtrace: popen(addr2line) failed

The assertion in cata::optional<islot_tool>::get() is failing, but I'm not sure what is triggering it.
@Night-Pryanik

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 5, 2019

Crash on game start?
@AMurkin

AMurkin reviewed Mar 5, 2019
View changes

src/player.cpp Outdated
return has_item_with( [&flag]( const item & it ) {
return has_item_with( [&flag, &need_charges]( const item & it ) {
if( need_charges ) {
return it.has_flag( flag ) && it.type->tool->max_charges ? it.charges > 0 : it.has_flag( flag );

This comment has been minimized.

Sign in to view
Copy link
@AMurkin

AMurkin Mar 5, 2019

Contributor

You need to check it.is_tool() probably. And what function does it.type->tool->max_charges perform here?

This comment has been minimized.

Sign in to view
Copy link
@Night-Pryanik

Night-Pryanik Mar 5, 2019

Author Member

Check for max_charges is a hack to check if this item requires charges to work.
@Night-Pryanik
Update player.cpp
Loading status checks…
4b3cc60
@Night-Pryanik

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 5, 2019

Added a sanity check for tools.
@kevingranade

kevingranade reviewed Mar 8, 2019
View changes

src/player.cpp Outdated
return has_item_with( [&flag]( const item & it ) {
return has_item_with( [&flag, &need_charges]( const item & it ) {
if( it.is_tool() && need_charges ) {
return it.has_flag( flag ) && it.type->tool->max_charges ? it.charges > 0 : it.has_flag( flag );

This comment has been minimized.

Sign in to view
Copy link
@kevingranade

kevingranade Mar 8, 2019

Member
Suggested change
return it.has_flag( flag ) && it.type->tool->max_charges ? it.charges > 0 : it.has_flag( flag );
return it.has_flag( flag ) && it.type->tool->max_charges ? it.charges > 0 : true;

This comment has been minimized.

Sign in to view
Copy link
@Night-Pryanik

Night-Pryanik Mar 10, 2019

Author Member

I guess this change caused this bug: #28619.
@kevingranade
Update player.cpp
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
d069775

@kevingranade kevingranade merged commit 9a41eeb into CleverRaven:development Mar 8, 2019
0 of 3 checks passed

0 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
gorgon-ghprb Build triggered for merge commit.
Details

@Night-Pryanik Night-Pryanik deleted the Night-Pryanik:fix-cellphones-with-zero-charge-still-showing-time branch Mar 9, 2019

davidpwbrown added a commit to davidpwbrown/Cataclysm-DDA that referenced this pull request Mar 9, 2019

@Night-Pryanik @davidpwbrown
Only charged electronic devices will show time and work as an alarm c… 
…locks (CleverRaven#28485)

* Only charged electronic devices will show time
* Added a special case for items that require charges to have some flag
* Alarm clocks require charges too to operate
3b64af2

davidpwbrown added a commit to davidpwbrown/Cataclysm-DDA that referenced this pull request Mar 9, 2019

@davidpwbrown
removed string initlializing to "" 
Update ru.motd (CleverRaven#28548)

Sets difficulty of removal for faulty CBMs (CleverRaven#28145)

* Creates Item for faulty_bionics to set difficulty of removal
* Defines and uses "abstract" : "bionic_general_faulty"

Only charged electronic devices will show time and work as an alarm clocks (CleverRaven#28485)

* Only charged electronic devices will show time
* Added a special case for items that require charges to have some flag
* Alarm clocks require charges too to operate

Adds NO_SIGHT flag to some furniture (CleverRaven#28488)

* Add NO_SIGHT to pillow fort
* Add NO_SIGHT to dumpster
* Moves HIDE_PLACE to correct position and add NO_SIGHT
ab85e50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.