-
Notifications
You must be signed in to change notification settings - Fork 999
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
Customizable capture requirements for ships #8506
Conversation
These changes were gone over during one of Derpy's streams, and everyone present agreed these were good and useful changes. That includes me. Here's my seal of approval. |
To go with the customisable capture requirements in endless-sky/endless-sky#8506 The Naltok capital ships will be unavailable to purchase or capture until part way through the story (both options will become available simultaneously), while the Xochira security is more of a lore change than a balance one.
I just reviewed the PR text (and the screenshots), not the code yet; the new mechanism looks nice. I like the idea that the capture tool can be used up in the capture attempt. The text "a tool is required to capture" sounds a bit strange to me (but I'm not a native speaker);
Some considerations:
|
I will note that we are VERY limited in what we can put in this UI at the moment. We have five lines of text max, with very little width to work with, and it doesn't even automatically word wrap longer lines. Without overhauling the boarding panel UI we can't have text as long as your suggestions, and such an overhaul would likely warrant its own PR.
Ships can't have "enablers" right now, and they can't be allowed to while the keywords are the same. Although I'm not sure what the behavior of a ship having a built-in enabler would be. Tools that are used for capturing are capable of breaking. How would a ship's built-in tool break? Would it be unbreakable? Would the desire to have a built-in capture tool not be met by just having an uninstallable outfit?
Outfits can only ever be tools for capturing at the moment, never for preventing hijacking. I designed this with locks and keys in mind. A ship can have one or many doors to enter. These doors come built into the ship, and you cannot add or remove doors (just as there are various attributes which we can't change on our ships). These doors are locked (the "capture class" on the ship), and each requires a certain key to open (the "capture class" on your outfits). Although opening a single door is enough to capture the ship (there are no doors behind doors), and no door has multiple locks, which does lead into your next point:
I'd rather there be a future enhancement to provide both OR and AND behavior (or some equivalent mechanics that causes multiple outfits to be required), instead of replacing OR with AND. Note that although a ship with multiple capture classes will be easier to capture with certain outfits, those outfits will also be harder to get your hands on. The purpose of the OR behavior is to provide a progression with capturing ships of the same type as well as ships of varying types. (That is, with better gear you can capture A better than you used to be able to, but you can also capture B when you used to be unable to.) Switching to AND behavior would only allow us to have progression of capturing ships of varying types. (That is, you either can or you can't capture A. Once you can capture A, it never gets easier.)
Yes and no. Although note that a ship can only become locked down after hand to hand combat has ceased, which means that the ship will have zero crew. I'm unaware of whether ships will try to repair friendlies that are empty husks, but if they do, it's not like the ship is suddenly functional now. It's still dead in the water, for one reason or another. |
This has been a long time coming. Looks good. I would like the option for an "And" in there too, but that's fine for a follow up PR. Anyway, thanks for working on this. Looking forward to having it in-game. |
Agreed, let's keep the texts as-is for now. This can go with an UI overhaul of the boarding panel in some future PR.
Agreed, that's why I suggested different keywords for the "lock" and for the "key" (as you nicely called them).
The ship would be destroyed, so using your ship as a (not 100% safe) capture-tool is a risky business. If the capture itself succeeds when the ship breaks, then in a future PR we could also have the player transferred to the captured ship. (But for now it just results in the player dying, as transferring players to other ships is currently not merged yet.) I can imagine that we even get "speciality capture ships" which only exist to act as capturing ships. (But that is likely going to be plugin stuff.)
If an uninstallable outfit gets destroyed, then I would also expect the ship to be destroyed. (But this would also be for a future PR, don't include it here.)
I understand the ideas, and I was thinking of an alternative design:
I feel that we don't really need the OR behavior if we make some other changes as well:
We can already have this progression with the AND behavior and only properties on the the keys; the player will get access to better keys over time. The "scriptkiddie keycard" typically breaks before it is even inserted into the card-reader of a cruiser, while the "admirals keycard" will have a 100% success rate against a cruiser, but gets used up when used once. (I would imagine the "admirals keycard" not to be for purchase in generic outfitters, but be given to the player in a specific capture mission.)
If we encode the success factors only in the keys, then we can also have both; Outfits can have multiple "capture keys", and newer outfits can provide better keys, so getting better outfits will give the player access to more keys, as well as access to better keys than before.
Agreed. Not an issue at this moment. |
The reason that the locks have all the same attributes as the keys is because although the same tool might be able to work on multiple ships, those ships might not all have the exact same lock model, and so different lock models might be more or less resistant against the same tool. Stepping away from the lock-and-key discussion, another goal with this PR was to create some method where we can tweak roughly how much money a player needs to put into a ship to capture it. If a ship has a 50% capture chance with a specific tool that costs 25k, then it'll take (I think) on average about 1.44 outfits to capture the ship. So using those metrics, we could say that the capture cost of this ship is effectively 36k. Now the capture cost of a ship should increase with the ship's base cost, so say we have another ship that costs twice as much as the first one, meaning the capture cost should be (at least about) twice as much. There's two things we could do to make that happen.
When you're talking about only two ships, neither approach really becomes the "obvious" approach to take. But we don't only have two ships in the game, we have over 200, with about 60 in human space alone (not counting Marauders). If we needed to have one tool for each ship in order to balance their capture costs, even if we made some ships require no tool, then that wouldn't just be cumbersome from a development perspective, but also a gameplay perspective in getting the right tool for the specific job. Therefore, I don't think it'd be the best to only limit success rates to the keys, as even if that does open us up in some areas, I think it also closes us off in other, more important ones. I guess to get to the heart of the matter, what actual benefit would we see from having AND behavior, or from having outfits act as locks, or ships with integral locks, compared to what this PR already provides?
|
We are discussing on different topics:
On the "one keyword" vs "different keywords": I see no downsides to not using different keywords (except maybe some rework on this PR). On "key-only" vs "key+lock": Some arguments in favor of "key+lock":
Just after posting I realized that there is another variant "lock-only", so I also added some arguments in favor of "lock-only":
Some arguments in favor of "key-only":
I still have a small preference for "key-only". On AND vs OR vs AND+OR: I feel that we should chose to implement either "AND" or "OR" behaviour in keys/locks, but not both. Keeping the system simple also has value. The "OR" and "AND" approaches are fundamentally very similar when it comes to ships having only 1 lock. The difference begins with ships with multiple locks; does each lock need to be opened, or is opening any lock sufficient? Some unique benefits that I see for the "OR" approach:
Some unique benefits that I see for the "AND" approach:
I still have a preference for "AND". On "which outfits provide locks": I agree that H2H outfits should not provide locks. It should probably be some new outfit like "computer security upgrade" or "reinforced bridge doors" that provides additional locks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea, especially how it would potentially combine with capture override to do say a plugin disabling this mechanic, for instance but also meaningfull exceptions, probably (shouldnt we make this a gamerule tho?)
Looks like it would work as advertised from what I could see of the code (but I'm on phone), I just noticed some things I think we should discuss.
capTools[name].emplace_back(sit.second, it.first); | ||
} | ||
|
||
// Sort capture tools be decreasing success chance, so that the best tool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Sort capture tools be decreasing success chance, so that the best tool | |
// Sort capture tools by decreasing success chance, so that the best tool |
child.PrintTrace("Skipping invalid capture class key with no value:"); | ||
const string &key = child.Token(0); | ||
double value = child.Value(1); | ||
if(key == "success") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since all elements are % between 1 and 0 shouldnt we have a check that the given value is in that range at the start, and a warning that it will default to 0/1 if it isnt between the two boundaries, if we dont completely disregard it?
Otherwise someone may write 50, imagining its % and get 100% applied instead (+ we're doing the same code at every line)
@@ -4224,6 +4231,65 @@ const vector<weak_ptr<Ship>> &Ship::GetEscorts() const | |||
|
|||
|
|||
|
|||
vector<pair<string, CaptureClass>> Ship::CaptureRequirements() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually I dont approve of putting comments in both the cpp and h but in this case since its done for other functions and can be a bit of a complex return type (especially for the second function) I'd add it here too?
// The capture classes of the ship being boarded. Stored in a list paired by | ||
// the capture class name and the capture class itself. The list is sorted by | ||
// an increasing success chance from the capture class. | ||
std::vector<std::pair<std::string, CaptureClass>> captureRequirements; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two feel a bit huge and use a pair + vector and map so we probably should make a small class combining it?
I thought we were discouraging the usage of pair to begin with given it contains unnamed data
We also have to explain a lot when a good + functions class name would do that everywhere we use it, I see that there are some differences for sure though so maybe its not easy because of that, but maybe logic could be moved to such a class, too?
Maybe its fine because the pair contains the name of the thing that is stored with it?
else if(!captureRequirements.empty()) | ||
{ | ||
// Other ships may require tools to capture that the player doesn't have. | ||
if(captureTools.empty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We dont seem to account for the case where the player has tools but none that work for this ship? Or do they all work on all ships?
const string &key = child.Token(0); | ||
double value = child.Value(1); | ||
if(key == "success") | ||
successChance = max(0., min(1., value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you can have a tool with 0% succes chance?
I assume that all attributes will be displayed, right? (Meaning we cant do pirates doing a sneaky one by selling it to you?)
It seems a bit weird because the boarding will happen as if you could do smth but you wont at the end?
Same thing for the ship having 0% cap chance itself, it's kind of weirdly uncapable in that case.
Do we just accept mis-use or should we put safegards/warnings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C++17 introduce std::clamp
I've removed myself as a reviewer: |
I'm thinking of redesigning this to a system which can be a bit more flexible, but not planning on getting to this any time soon. Not considering this PR orphaned, but I am closing it since it's not moving any time soon and what's here will likely be redone. |
Feature: This PR implements the feature request detailed and discussed in issue #2948.
Resolves #2948
Feature Details
This PR adds the ability for ships to have requirements in order to be captured, and for specific outfits to be able to meet those requirements. This is done via a new "capture class" attribute of ships and outfits.
This PR only adds the mechanic for this to be possible, and does not implement its use into any ships or new outfits.
Syntax
"capture class" <name>
: The name of the capture class that the ship or outfit is in. When attempting to capture a ship, you must have an outfit with a capture class that matches the ship. If a ship has no capture class, then no outfit is required to capture it. A ship or outfit can have multiple capture classes, allowing one outfit to hijack multiple types of ships, or one ship to be susceptible to multiple types of outfits.A capture class can have any of the following five children. If any of them is not specified, then a default value of 0 is used. If a ship or outfit has multiple capture classes, then each can be given a different set of children.
"success" <odds#>
: A value from 0 to 1 that denotes the odds of success. For the outfit, this means the chance that it successfully hijacks the ship. For the ship, this means the chance is successfully fends off a hijacking attempt. The odds of a successful hijacking are then(outfit success) * (1 - ship success)
."self destruct" <odds#>
: A value from 0 to 1 that denotes the odds of the ship self destructing after a failed hijacking attempt. If both the outfit used and the ship have this capture class attribute, then both will roll independently to determine if the ship explodes. That is, the odds of a ship self destructing are1 - (1 - outfit self destruct) * (1 - ship self destruct)
."lock down" <odds#>
: A value from 0 to 1 that denotes the odds of the ship locking down after a failed hijacking attempt. A ship which has locked down can no longer have any hijacking attempts made against it. This is a weaker form of self destruct, preventing you from capturing the ship but still allowing you to plunder from it. The game checks if a ship has locked down after checking if the ship has self destructed. As with self destruct, if both the outfit and ship have this attribute then both roll to determine if the ship should lock down."success break" <odds#>
: A value from 0 to 1 that denotes the odds of the outfit breaking on a successful hijack. An outfit breaking means you lose it, with one outfit of the type used being removed from your flagship. As above, both outfit and ship having this attribute means that both roll."failure break" <odds#>
: A value from 0 to 1 that denotes the odds of the outfit breaking on a failed hijack. As above, both outfit and ship having this attribute means that both roll.Gameplay Behavior
Aside from how each attribute functions as described above, there are some additional gameplay behavioral notes worth making:
"self destruct"
attribute to only run on the first capture attempt. That is, defending and pulling back then deciding to attempt to capture gain won't be able to cause a self destruct on the second attempt.UI Screenshots
Boarding a ship with no capture requirements. Looks the same as before, and killing the crew captures the ship.
Boarding a ship with capture requirements where you have the correct tool. You're told that a tool is required, and clearing the crew moves on to a "hijacking" phase of capturing. If you attempt to hijack the ship, you're told what tool was used and the outcome of the attempt. If your tool succeeds then the ship is yours.
If you lack the tools to capture the ship, then you won't be able to engage in hand to hand combat. If you run out of tools, you'll be unable to make more capture attempts unless you return with more tools.
If the ship self destructs then you're kicked out of the boarding panel and met with a dialog.
If the ship locks down then no more capture attempts can be made. Leaving and returning to the ship, even reloading the save, it will remain uncapturable.
Usage Examples
The following could be a way to create a two-tiered capture system for automata ships. Different automata ships could be given different self destruct or lock down rates, or even different success rates if that was desired.
Testing Done
Created a mission with a derelict Headhunter, Aerie, and Cruiser that spawned.
These were the stats on the outfits tested:
Automated Tests Added
N/A
Performance Impact
N/A