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

Civillian and Medical ship Holopad update #2800

Merged
merged 4 commits into from
Jan 31, 2025
Merged

Conversation

Cheackraze
Copy link
Member

@Cheackraze Cheackraze commented Jan 28, 2025

About the PR

Adds holopads to the 42 civilian/medical ships. Missing are, Caduceus(pending replacement any day now), Barge (already implemeneted), Pioneer(changes incoming) and Camper (no room-need design consideration)

Why / Balance

#2795 (comment)

How to test

Spawn the ships in your favorite method, in game or dev or mapping or testing, and see the glorious holopads

Media

Requirements

Breaking changes

Changelog

🆑

  • tweak: Added long-range holopads to most civillian ships

@dustylens
Copy link
Contributor

There is a PR out for the Comet #2690 with a holopad already added. Does that seem acceptable? To prevent a conflict.

@dvir001 dvir001 requested review from arimah and MagnusCrowe January 29, 2025 20:00
@github-actions github-actions bot added the S: Needs Review This PR is awaiting reviews label Jan 29, 2025
@dvir001
Copy link
Contributor

dvir001 commented Jan 29, 2025

Is any maptainer testing this or I'll need to? 👀

@arimah
Copy link
Contributor

arimah commented Jan 29, 2025

I'll get on with testing it tomorrow. :>

Copy link
Contributor

@arimah arimah left a comment

Choose a reason for hiding this comment

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

I've gone through all 42 ships, and the changes are pretty minimal. Looking promising so far!

Comments:

  • Bulker: I don't love the placement of the holopad or the station records computer, but I will approve both. I'm not sure how else one could reasonably arrange things within the extremely limited available space.
  • Comet: There is an open PR to revamp the Comet (Comet Defib Update #2690). I will merge this one first, as the other PR is enough of a rework that it'll probably take some time to review it properly, and I'd like to get holopads sorted sooner rather than later.
  • Crescent, Harbormaster, Loader, Lyrae: The holopad is on top of a vent. Depending seemingly on the game's whims, though the holopad is visually on top, sometimes the vent "steals" mouse focus, forcing you to carefully click on the part of the holopad that's outside the vent. It's not great. I'm prepared to allow it for now, but I'd like to find a different, better long-term solution.
  • McDelivery: This seems to have lost its station records computer. Are we no longer going to require those on tiny daughterships?

Ships not touched by this PR:

  • Akupara
  • Construct

Please verify that these are intended to be excluded. :)

@dvir001
Copy link
Contributor

dvir001 commented Jan 30, 2025

Record computer needed to track owner

@arimah
Copy link
Contributor

arimah commented Jan 30, 2025

Record computer needed to track owner

Are you sure about that? 🤔

@arimah arimah mentioned this pull request Jan 28, 2025
89 tasks
@Tych0theSynth
Copy link
Contributor

I've recently added a holopad to the Bodkin on my open PR to update it #2640

@Houtblokje
Copy link
Contributor

  • Crescent, Harbormaster, Loader, Lyrae: The holopad is on top of a vent. Depending seemingly on the game's whims, though the holopad is visually on top, sometimes the vent "steals" mouse focus, forcing you to carefully click on the part of the holopad that's outside the vent. It's not great. I'm prepared to allow it for now, but I'd like to find a different, better long-term solution.

That does seem bad. I understand maybe the harbormaster and Loader not having enough space to put the holopad, but is there really not enough space on the lyrae and the crescent (with crescent being one of the biggest ships currently in the game, and lyrae having that pretty large open lobby/hallway)?

@dvir001
Copy link
Contributor

dvir001 commented Jan 31, 2025

Record computer needed to track owner

Are you sure about that? 🤔

I mean you see the ship owner on it

@Houtblokje
Copy link
Contributor

Record computer needed to track owner

Are you sure about that? 🤔

I mean you see the ship owner on it

I mean, only half of the time iirc but thats another issue.

Copy link
Contributor

@arimah arimah left a comment

Choose a reason for hiding this comment

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

Outstanding issues resolved in conversation on Discord. Over-vent holopads will be accepted for now. Akupara has a PR that reworks it (#2816), will let that introduce the holopad. Construct is skipped for now, will be included in a later batch. All ships approved. Let's get this show on the road. :)

@arimah arimah merged commit 6783878 into master Jan 31, 2025
12 checks passed
FrontierATC added a commit that referenced this pull request Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Map-Shuttle Map - Shuttle No C# S: Needs Review This PR is awaiting reviews size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants