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

Refactors AI / camera eyes and slows holopad holograms to walk speed #25078

Merged
merged 31 commits into from
Jan 17, 2025

Conversation

asciodev
Copy link
Contributor

@asciodev asciodev commented Apr 11, 2024

What Does This PR Do

This change completes a refactor of AI eyes, which were previously used by xenobio consoles, syndicate and abductor camera consoles, shuttle docking computers, holograms, and, of course, the AI. Duplicated logic has been extracted to an abstract base mob, /mob/camera/eye, from which new types for each of the above now derive. Functionality is largely the same, with only a few minor cosmetic differences (i.e. camera eyes are now appropriately named given their type and user), as well as a quality-of-life enhancement for holograms, slowing their movement speed to base run speed to prevent users from accidentally zooming out of calls. A toggle for switching between this behavior and full speed has also been added to AI Commands, called "Toggle Fast Holograms", disabled by default.

Why It's Good For The Game

  • Duplicated logic sporadically spread across files is hard to follow; maintainability is good.
  • Holograms are currently nearly unusable by crew due to how easy it is to zoom out of holopad range at AI eye speeds. This change fixes that.

Testing

  • Using a test server, I logged in as a Head of Personnel, VV'd the nearest holopad, and set force_answer_call to 1. I then called the Bridge, and upon being auto-answered, I was given sight and control over a hologram on the bridge, as expected. It was no longer zoomy. I left the holopad range and was disconnected from the call, regaining sight and control over my character. I was also able to exit a call using the associated action button.
  • I then made myself an AI, and verified that I could still move around the station, switch various cameras, view my core, turn on and off camera lights, turn on and off acceleration, and set and use saved camera locations. I activated a holopad and moved around, which worked as expected, and identically in function to before this PR, except no longer zoomy. Upon leaving range of nearby holopads, my hologram disappeared, as expected, and my speed returned to normal AI eye speed.
  • I wiped my core and sent myself back to lobby, and joined as a Solar Federation General. I walked to the specops navigation computer on the port-side shuttle and verified that I could still create a custom shuttle destination, and that the turf overlays designating valid/invalid destination areas still showed up, and that I could not designate invalid transit locations.
  • I spawned an advanced camera console (the kind that appears in the Syndicate Research Base) and used it. I was able to jump to various cameras and move around.
  • I made myself an abductor scientist and teleported to the abductor ship. I used the abductor camera console, and was able to jump to various cameras, move the camera around, set teleport targets, teleport myself in, teleport Pun Pun in, and my vision was blocked by static where there were no active cameras.
  • I teleported myself to Xenobiology, and used one of their consoles. I was able to pick up slimes, place monkeys, and exit the camera via action button. The camera was still confined to the allowed xenobiology area, and attempting to jump to areas that were not in the allowed area still bounced back to xenobiology.

Changelog

🆑 Ascio
add: Camera eyes not belonging to the AI now have descriptive names, allowing easier orbiting
add: Added a "Fast Holograms" toggle to AI commands, which sets holopad holograms to camera eye speed (disabled by default)
tweak: Slowed holopad holograms to base run speed by default to prevent accidental holocall hangups
/:cl:

asciodev added 7 commits April 7, 2024 15:20
Camera Eyes previously had duplicated logic across several files. This
change uncooks the spaghetti. Additionally, half-baked support for TG's
multicam feature has been removed, as it was not functional or in use.
This change completes a refactor of AI eyes, which were previously used
by xenobio consoles, syndicate and abductor camera consoles, shuttle
docking computers, holograms, and, of course, the AI. Duplicated logic
has been extracted to an abstract base mob, /mob/camera/eye, from which
new types for each of the above now derive.

Functionality is largely the same, with only a few minor cosmetic
differences (i.e. camera eyes are now appropriately named given their
type and user), as well as a quality-of-life enhancement for holograms,
slowing their movement speed to base run speed to prevent users from
accidentally zooming out of calls.
The acceleration toggle was broken in the camera eye refactor, as
previously the boolean was stored on the AI rather than its eye. This
change fixes that.
With the camera eye refactor, the syndicate advanced camera consoles
lost the ability to view maintenance tunnels and other areas without
active cameras, seeing static in their place instead (as all other
cameras do). This change reinstates the original behavior.
@asciodev asciodev changed the title Camera eye refactor merge Refactors AI / camera eyes and slows holopad holograms to walk speed Apr 11, 2024
@ParadiseSS13-Bot ParadiseSS13-Bot added the -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally label Apr 11, 2024
code/__DEFINES/mob_defines.dm Outdated Show resolved Hide resolved
code/game/machinery/hologram.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye/shuttle_navigator.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye/syndicate.dm Outdated Show resolved Hide resolved
code/modules/mob/living/silicon/ai/ai_mob.dm Outdated Show resolved Hide resolved
code/modules/mob/living/silicon/ai/ai_mob.dm Outdated Show resolved Hide resolved
code/modules/mob/mob_movement.dm Outdated Show resolved Hide resolved
General minor code quality improvements suggested by GDNgit

Co-authored-by: GDN <[email protected]>
@Daylight2
Copy link
Contributor

I personally liked being able to zoom away as AI if you were urgently needed as whilst in a holopad, although I suppose that run speed isn't that slow. AI players are probably used to the high speed, however, and the fix to messing up is just triple clicking for them, instead of requesting a whole new call.

@asciodev
Copy link
Contributor Author

After some discussion on discord I agree that the change could be frustrating for many AI players who are used to the zoominess and have mastered it. Although the AI Commands tab is already rather bloated, I will add a toggle for fast holograms.

Rename parameter names to avoid src accesses, remove an ambiguous and
unused mob_define and holopad range variable from a previous WIP, change
the for loop in /mob/camera/eye/relaymove to a for-to loop, and change
the chat message warning, sent when an AI Eye is created on an AI that
already has one, to a stack trace
@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting review This PR is awaiting review from the review team and removed -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally labels Apr 12, 2024
@Henri215 Henri215 added Refactor This PR will clean up the code but have the same ingame outcome Tweak This PR tweaks something ingame labels Apr 12, 2024
@github-actions github-actions bot added the Merge Conflict This PR is merge conflicted label Apr 12, 2024
@github-actions github-actions bot added Merge Conflict This PR is merge conflicted and removed Merge Conflict This PR is merge conflicted labels Apr 13, 2024
Copy link
Contributor

github-actions bot commented May 4, 2024

This pull request seems to be stale as there have been no changes in 14 days, please make changes within 7 days or the PR will be closed. If you believe this is a mistake, please inform a development team member on Discord.

@github-actions github-actions bot added the Stale This PR has been left inactive and requires an update. label May 4, 2024
asciodev added 2 commits May 5, 2024 08:37
Previously, the relaymove proc for hologram eyes was redundant and
nearly impossible to read. It has been separated out into a few
different named procs, and has had its use of `spawn` removed.
Copy link
Contributor

@Drsmail Drsmail left a comment

Choose a reason for hiding this comment

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

Really like what you did here, didn't have time to chek logick, will give it a second look later.

code/game/machinery/hologram.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye/hologram_eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye/xenobio_eye.dm Outdated Show resolved Hide resolved
code/modules/mob/living/silicon/ai/ai_mob.dm Outdated Show resolved Hide resolved
@github-actions github-actions bot removed Merge Conflict This PR is merge conflicted Stale This PR has been left inactive and requires an update. labels Jan 1, 2025
Copy link
Contributor

@FunnyMan3595 FunnyMan3595 left a comment

Choose a reason for hiding this comment

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

New description/changelog LGTM.

@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting merge This PR is ready for merge and removed -Status: Awaiting review This PR is awaiting review from the review team labels Jan 4, 2025
code/datums/holocall.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye.dm Outdated Show resolved Hide resolved
code/modules/mob/camera/eye/ai_eye.dm Outdated Show resolved Hide resolved
asciodev and others added 3 commits January 9, 2025 10:33
Changed a number of camera eye-related variables to camel_case style,
added appropriate autodoc comments, as per code review. Also removed an
unused cameranet function, modified the call signature of a cameranet
function to be more semantic, and changed a qdel-on-initialize in camera
eyes to return INITIALIZE_HINT_QDEL instead.

Co-authored-by: Luc <[email protected]>
@asciodev
Copy link
Contributor Author

asciodev commented Jan 9, 2025

I mass-replaced everything that I could find related to this system from PascalCase to camel_case, except for TGUI data prop names. I also added autodoc comments to just about every variable and function related to camera eyes. I tested as per the PR description and no unexpected behavior or errors cropped up.

@Burzah Burzah requested a review from lewcc January 10, 2025 20:08
@Burzah Burzah added this pull request to the merge queue Jan 17, 2025
Merged via the queue into ParadiseSS13:master with commit 93ed0f0 Jan 17, 2025
17 checks passed
@TravisAngeI
Copy link

Something like, mildly annoying but super small that I doubt can be fixed, is that if the AI moves in their camera-view, and I'm pretty sure it's as a result of this PR, and they let go of the key while the game is still trying to move them to that tile, it does like a weird, slow movement, it's mostly a visual thing.

Just figured I would point it out (this is kinda nitpicky), it has no real harm but I was maybe hoping to see if there was possibility of tweaking it to make it not do that? Not sure how code worky.

It does this to holopads aswell, much more noticeably (see second video if they both attach properly)

302e19a41876a0fd5a4ee4db98d51826.mp4
2bec9d631209a889f6af56fae439ab84.mp4

asciodev added a commit to asciodev/Paradise that referenced this pull request Jan 18, 2025


PR ParadiseSS13#25078 introduced a few bugs related to AI eyes, mechs, and
holograms. This change fixes those bugs. Now, entering a mech with an
active hologram eye will first release that eye before granting control
of the mech. Double clicking on a turf as an AI while in a mech will no
longer unstick the camera from the mech. When exiting a mech, the AI
will now properly refocus on its core instead of on the spot that it
first entered the mech. AI holograms no longer receive normal audio
near their holopads, receiving only holopad-relayed speech now instead.
AIs will no longer be able to activate a holopad while occupying a mech.
github-merge-queue bot pushed a commit that referenced this pull request Jan 26, 2025
* Fixes #28022: mecha/AI/hologram bugs from #25078

PR #25078 introduced a few bugs related to AI eyes, mechs, and
holograms. This change fixes those bugs. Now, entering a mech with an
active hologram eye will first release that eye before granting control
of the mech. Double clicking on a turf as an AI while in a mech will no
longer unstick the camera from the mech. When exiting a mech, the AI
will now properly refocus on its core instead of on the spot that it
first entered the mech. AI holograms no longer receive normal audio
near their holopads, receiving only holopad-relayed speech now instead.
AIs will no longer be able to activate a holopad while occupying a mech.

* Update code/_onclick/ai_onclick.dm

Co-authored-by: Luc <[email protected]>
Signed-off-by: Burzah <[email protected]>

---------

Signed-off-by: Burzah <[email protected]>
Co-authored-by: Burzah <[email protected]>
Co-authored-by: Luc <[email protected]>
@JackSage JackSage mentioned this pull request Jan 28, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Status: Awaiting merge This PR is ready for merge Refactor This PR will clean up the code but have the same ingame outcome Tweak This PR tweaks something ingame
Projects
None yet
Development

Successfully merging this pull request may close these issues.