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

Make eBooks (Smartphone, Tablet, ...) only turn on their light in darkness #59261

Merged

Conversation

Nyghtrid3r
Copy link
Contributor

@Nyghtrid3r Nyghtrid3r commented Jul 14, 2022

Summary

Bugfixes "Make eBooks (Smartphone, Tablet, ...) only turn on their light in darkness"

Purpose of change

Fixes #55983

Describe the solution

I added a check for the player avatar's current visibility before the screen / flashlight is turned on (see below).

Also, we should rework the flashlight app for smartphones to work like an actual flashlight and distinguish between it and a lit up screen more concisely. The default action should be just turning the screen on as it is with the laptop and tablet.

Describe alternatives you've considered

We could leave it as it is for now, as this is just a temporary fix regarding smartphones. Reading eBooks should not require the flashlight at all, as the light from the screen should be enough to read (although I suppose reading from a device in darkness is bad for your eyes, so maybe our survivor is just very responsible).

I also considered making the flashlight turn off on its own after reading, but I haven't figured out how to do that yet.

Testing

1.) Spawned in a skill book, smartphone, as well as a tablet + battery
2.) Read the book once to learn of the contents
3.) Stored the book in the devices
4.) Moved to a bright spot, read stored book with each device
--> Tablet screen / smartphone flashlight did not turn on
5.) Move to a dark spot, read stored book with each device
--> Screen and flashlight DID turn on, could still read book with the tablet (see below for smartphone)

Additional context

Change:
image

For some reason, the smartphone's flashlight is not enough to read, as it does not light up the tile which the survivor is standing on. Wielding it, like you'd do with an actual flashlight, does not work either. However, that also happened during testing before I made the changes, so that must be an unrelated bug. Once that is worked out, this should also work with smartphones.

This is my first PR, pls no bully

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) labels Jul 14, 2022
@Nyghtrid3r Nyghtrid3r changed the title Make eBooks (Smartphone, Tablet, ...) only turn on their flashlight in darkness Make eBooks (Smartphone, Tablet, ...) only turn on their light in darkness Jul 14, 2022
@Nyghtrid3r Nyghtrid3r marked this pull request as ready for review July 14, 2022 13:59
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jul 15, 2022
@PatrikLundell
Copy link
Contributor

There are lots of remaining bugs causing effects to delay one turn, in particular relating to visibility, so if you're testing turning things on/off, you may want to move one tile in whatever direction (passing a turn usually doesn't work) to trigger reevaluations.

@Nyghtrid3r
Copy link
Contributor Author

Nyghtrid3r commented Jul 15, 2022

There are lots of remaining bugs causing effects to delay one turn, in particular relating to visibility, so if you're testing turning things on/off, you may want to move one tile in whatever direction (passing a turn usually doesn't work) to trigger reevaluations.

Ah damn it. I should have known that, I've observed this before while playing the game. I'll try that as well, but I'm debating whether or not we should set the default use action of the smartphone to "You turn on the screen" (like how it is with the Tablet) and have the flashlight be a separate use option. At least I think that's the best solution, because that properly fixes this bug. Problem is, I don't think an item can have multiple "transform" type uses. I'm not sure how I could implement that.

I'm gonna wait for some more input on this, then I'll maybe adjust this or make another PR. Right now I'm confused about that test failing.

@anothersimulacrum
Copy link
Member

It's an RNG test failure, it probably just needs a kick.

@Inglonias
Copy link
Contributor

Yeah, smartphone flashlights are currently not working at all (#59373)

@Fris0uman Fris0uman merged commit e4ce417 into CleverRaven:master Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smartphone turns on flashlight when reading books
5 participants