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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds camera modes + photographer trait uses #8386

Merged
merged 20 commits into from Jun 15, 2020

Conversation

Hopekz
Copy link
Contributor

@Hopekz Hopekz commented May 4, 2020

Adds modes to the camera.

What this means:
You can set your camera to shoot in standard mode or description mode by using the camera in your hand.

In standard mode, the camera will not prompt you if you want to customize the photo and in description mode, it gives you that annoying popup asking you if you want to customize your photo.

Edit 2:
Added a better examine (now tells you that the camera is empty when there are 0 pictures left)
You now have to be close to the camera to look inside of it to see the amount of pictures left for added camera immersion!

Edit 3:
Added more use for the Photography trait.

When you have the photography trait you get the following information when you examine:

the exact number of photos left in the camera
the lens setting of the camera
the mode the camera is in

EDIT 4:
I managed to recreate the bug after multiple playtests so I am no longer claiming it is fixed.

Changelog

馃啈 Hopek
rscadd: The camera has modes. You can now shoot in a standard mode where it won't give you an annoying popup box and a description mode where it prompts you.
rscadd: Expanded the usage of the photographer trait for the Camera.
/:cl:

@yogstation13-bot yogstation13-bot added the Feature This adds new content to the game label May 4, 2020
@Hopekz Hopekz added the Fix This fixes an issue. Please link issues in fix PRs label May 4, 2020
@Hopekz
Copy link
Contributor Author

Hopekz commented May 4, 2020

Update:
Added future proofing to include a case that doesn't exist yet if someone were to make a camera that doesn't have the ability to use descriptive mode (if it can't change descriptions)

@Hopekz Hopekz changed the title Fixes Camera bug + adds camera modes Fixes Camera + adds camera modes May 4, 2020
@Hopekz
Copy link
Contributor Author

Hopekz commented May 4, 2020

Edit 3:
Added more use for the Photography trait.

When you have the photography trait you get the following information when you examine:

  • the exact number of photos left in the camera
  • the lens setting of the camera
  • the mode the camera is in

@Hopekz Hopekz changed the title Fixes Camera + adds camera modes Fixes Camera + adds camera modes + photographer trait uses May 4, 2020
@MegaEmpirical
Copy link

Fuck yeah.

@alexkar598 alexkar598 added the Awaiting - Action - Maintainer This PR is awaiting an action from a maintainer label May 6, 2020
@Hopekz Hopekz added the Awaiting - Merge This PR is ready for merge label May 6, 2020
@swissloaf swissloaf removed the Awaiting - Merge This PR is ready for merge label May 7, 2020
@Hopekz Hopekz added the Awaiting - Merge This PR is ready for merge label May 7, 2020
@Hopekz
Copy link
Contributor Author

Hopekz commented May 7, 2020

@swissloaf stop fucking with our labels or you'll get that privilege removed .
This is the 2nd PR where you added or removed the ready label it's for our organization

@swissloaf swissloaf removed the Awaiting - Merge This PR is ready for merge label May 7, 2020
@swissloaf
Copy link
Contributor

swissloaf commented May 7, 2020

This aint approved change

@Hopekz Hopekz added the Awaiting - Merge This PR is ready for merge label May 7, 2020
@alexkar598 alexkar598 removed the Awaiting - Merge This PR is ready for merge label May 11, 2020
@Hopekz
Copy link
Contributor Author

Hopekz commented May 12, 2020

@alexkar598 you may have forgotten to submit your review with suggestions on why you believe this isn't "ready".

@alexkar598
Copy link
Member

It's not "ready" because you cant review your own code

@Hopekz
Copy link
Contributor Author

Hopekz commented May 12, 2020

It's not "ready" because you cant review your own code

I was not aware the "ready" tag had any real meaning outside of organization since we need to perform a final review before merging.

In typical non self merge circumstances; I am fully capable of performing my own review as long as the review itself is not the sole reason for merge.

You have stated yourself that the "ready" tag has no further meaning outside of a comment approval.
It does not mean that the HC or maintainer that looks in to this in the future will bypass their code review as performing a code review before a merge should be mandatory (if it isn't already).

If we are expected to bypass our final review on PR's marked as "ready" we need to communicate this out better so everyone is on the same page.

Copy link
Member

@alexkar598 alexkar598 left a comment

Choose a reason for hiding this comment

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

Use defines instead of directly using strings for the mode

code/modules/photography/camera/camera.dm Outdated Show resolved Hide resolved
code/modules/photography/camera/camera.dm Outdated Show resolved Hide resolved
@alexkar598 alexkar598 added Awaiting - Action - Author This PR is awaiting an action from the author and removed Awaiting - Action - Maintainer This PR is awaiting an action from a maintainer labels May 14, 2020
@Hopekz
Copy link
Contributor Author

Hopekz commented May 15, 2020

Use defines instead of directly using strings for the mode

@alexkar598 done

@Hopekz Hopekz added Awaiting - Action - Maintainer This PR is awaiting an action from a maintainer and removed Awaiting - Action - Author This PR is awaiting an action from the author Fix This fixes an issue. Please link issues in fix PRs labels May 15, 2020
@Hopekz Hopekz changed the title Fixes Camera + adds camera modes + photographer trait uses Adds camera modes + photographer trait uses May 15, 2020
saved the codebase
Copy link
Member

@alexkar598 alexkar598 left a comment

Choose a reason for hiding this comment

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

Il have to admit im not a fan of making cameras even more useless to just about 90% of people

code/modules/photography/camera/camera.dm Outdated Show resolved Hide resolved
code/modules/photography/camera/camera.dm Outdated Show resolved Hide resolved
Hopekz and others added 2 commits June 8, 2020 12:53
Thanks Alex!

Co-authored-by: alexkar598 <25136265+alexkar598@users.noreply.github.com>
@Hopekz
Copy link
Contributor Author

Hopekz commented Jun 8, 2020

Il have to admit im not a fan of making cameras even more useless to just about 90% of people

The only solution I can see is to change the photographer trait to "has a camera" and just make everyone have equal proficiency with the tool or removing this as a trait. Being a photographer means you get expertise in this specific area in the game.

With these trait changes, picture-taking remains unaffected but you get all the complicated information on examination similar to someone knowing how to use the tool proficiently.

In my opinion, we should expand this further to other areas of the game like guns because right now you are aware of the exact number of bullets in a nonclear magazine just by looking at it which is weird. This should be reserved for people that are more knowledgeable in these items but that is a separate PR.
Similar to this you're aware of the exact number of pictures left in a camera just by looking at it.
To me, it seems like this is something that proficiency would let you know.

Also if you're planning to close this PR just request the edit of it being removed because the main focus of this PR is adding the camera modes so it doesn't spam the popup every time you take a picture.

@alexkar598 alexkar598 merged commit 2af9791 into yogstation13:master Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting - Action - Maintainer This PR is awaiting an action from a maintainer Feature This adds new content to the game
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants