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

Radio jammer update! #25912

Merged
merged 39 commits into from
Apr 25, 2024

Conversation

beck-thompson
Copy link
Contributor

@beck-thompson beck-thompson commented Mar 7, 2024

About the PR

Radio jammer now has selectable power levels and a battery level indicator light!

Why / Balance

The radio jammers default range of 8 was relatively large for most situations. Expanding its functionality with the three selectable ranges gives it a lot more utility. Now, you can privately threaten CMO without the rest of medical losing radio access! Or, you could crank up the power to max and, for a short period of time, completely black out all of security from hearing the HOPs cries for help. This will give antagonists more control (And ability to mess up!) in executing plots involving the radio jammer!

Technical details

The main thing that was implemented was adding a OnGetVerb function that adds the menu for selecting the power level. I also created two functions GetCurrentWattage and GetCurrentRange because now that they can change the jammer system needs a way to determine what the current range / wattage is. For the battery level indicator, I just switch the sprite to a new level depending on the battery power.

Media

radiojammer.webm
batt.webm

Changelog

🆑 Beck Thompson

  • tweak: Radio jammer now has 3 selectable power operating levels and a battery level led indicator!

@github-actions github-actions bot added the Changes: Sprites Changes: Might require knowledge of spriting or visual design. label Mar 7, 2024
Copy link
Contributor

github-actions bot commented Mar 7, 2024

RSI Diff Bot; head commit 84cf50f merging into 5b0c3ec
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Objects/Devices/jammer.rsi

State Old New Status
jammer Modified
jammer_high_charge Added
jammer_low_charge Added
jammer_medium_charge Added

Edit: diff updated after 84cf50f

@beck-thompson
Copy link
Contributor Author

This is the new PR after I nuked the old one: #25546

@metalgearsloth Here are the commits that fix the stuff you wanted! Let me know if its right.

fc50e37
f93e85d
28e29b4
22f061a

Copy link
Contributor

@Flareguy Flareguy left a comment

Choose a reason for hiding this comment

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

The radio jammer absolutely could use a resprite, but I'm not a very big fan of this.
The dish looks very out-of-place on the radio jammer. I'm not 100% confident why that is, but there are a few possible culprits from what I can tell:

  • The coloration and shading does not fit with the rest of the item
  • The rim of the dish is too contrast-y and should probably just use colors picked from or just above the radio jammer's body
  • The dish is probably way too big

The dish is also fairly out of style feeling, probably mainly due to its hueshifting and lack of outlines in some spots. For hueshifting, you should probably reduce it to somewhere around 1-2 hue difference per color increment at most, or just opt to not hueshift it at all.

@beck-thompson
Copy link
Contributor Author

Thanks for the feedback! Sounds like I totally missed the mark on the sprite (Fair enough 😆). It took me a while to make as I don't have much experience (one udemy pixel art class a few years ago). Next week is finals week and I just don't have the time to make the requested changes as it will probably take a few iterations.

I'll just remove it and I (Or someone else!) can update it later.

@beck-thompson beck-thompson requested a review from Flareguy March 8, 2024 01:11
@github-actions github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted label Mar 8, 2024
@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Mar 14, 2024
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@metalgearsloth metalgearsloth added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Apr 14, 2024
@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Apr 15, 2024
@Dueu
Copy link

Dueu commented Apr 15, 2024

Ain't a way to code it so it doesn't block syndie comms?

@beck-thompson
Copy link
Contributor Author

Maybe but I personally think that's a little weird! It makes sense it that it blocks ALL communication / radio transmissions. What could be interesting is adding a new menu where you can choose what channels to block (E.g block sec and command coms but leave the rest open) but that's out of the scope for this PR (Also from what I heard the radio code is kind of a mess so it might not be as easy as you'd expect?).

Comment on lines 27 to 28
[Dependency] private readonly SingletonDeviceNetServerSystem _singletonServerSystem = default!;
[Dependency] private readonly SharedJammerSystem _jammerSystem = default!; // For all the helper functions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just have the server system inherit the shared one instead of running 2 different systems ingame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I fixed this now!

@metalgearsloth metalgearsloth merged commit d3b1178 into space-wizards:master Apr 25, 2024
11 checks passed
@beck-thompson
Copy link
Contributor Author

beck-thompson commented Apr 25, 2024

Yay! Thanks to everyone who helped / reviewed this! Ya'll put a a lot of effort to help me and I appreciate it :)
(I learned quite a lot 😆)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: Sprites Changes: Might require knowledge of spriting or visual design. S: Needs Review Status: Requires additional reviews before being fully accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants