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

Fine-grained Illegal/Atrocity Control (was Parrot and Author governments enforce only Human and Hai systems.) #5141

Closed
wants to merge 7 commits into from

Conversation

Eloraam
Copy link
Contributor

@Eloraam Eloraam commented Jul 2, 2020

Original Motivation

"Cap'n Pester" often shows up and scans the user. Normally this is fine, but if the player has any outfits with the "atrocity" property set, then suddenly they're being attacked by a Quarg Wardragon that just sorta shows up out of nowhere. That's a bit much even for such a terrible outfit.

New Approach

I added a new "legality" block to define how individual governments react to a class of outfits or mission cargos. They look like this:

legality "name"
    default illegal 1000
    default atrocity
    atrocity
        government "Republic"
    illegal 20000
        government "Free Worlds"

The change is entirely backwards-compatible, but now in missions you can say "illegal name" and in outfits you can say either "illegal name" or "atrocity name", depending on how you want it to show up in the outfitter. Now there is no change to stock gameplay of any sort, or to gameplay of plugins that don't use the new feature.

Testing Done

Ran a savegame unmodified and tested both legal and illegal missions and outfits. Tried again with outfits from a plugin that uses the new feature as well.

How to test

Start a regular mission, visit Sol, get scanned. Nothing. Start an illegal mission, visit Sol, get scanned - fined. Start an atrocious mission, visit Sol, get scanned - dead. Same with outfits and outfits in cargo.

@tehhowch tehhowch added the unlikely An enhancement or other change that is lowest priority label Jul 2, 2020
@tehhowch
Copy link
Collaborator

tehhowch commented Jul 2, 2020

This (Cap'n Hector's ES imitation fining you and in general being a harassment to "illegal" play) is intended behavior.

Plugins that add atrocity outfits are more than welcome to also modify the Author and Parrot governments to disable their ability to fine, or to add enforcement regions to limit the scope in which they fine

@Eloraam
Copy link
Contributor Author

Eloraam commented Jul 2, 2020

The fine is fine, but he goes full hostile if you use the "atrocity" property. Is that also intended?

Is "atrocity" never meant to be used in stock? Is this the intended behavior if content ever used it?

I'm happy to change approach if there's a more as-intended way of implementing it, or closing if it's working as intended.

@tehhowch
Copy link
Collaborator

tehhowch commented Jul 2, 2020

Yes, the atrocity property invokes a death sentence for the player. If the player is landed and caught, they will promptly die. If in flight (e.g. scanned with "atrocity" goods/outfits or attempting planetary domination), then the player's relationship with that government becomes immediately hostile.

@Zitchas
Copy link
Member

Zitchas commented Jul 2, 2020

"Atrocity" is intended to only be put on things that are so terrible that authors and responsible governments have included it on "destroy all instances upon discovery" lists. Rather like the "Omega Particle" in Star Trek lore.

@Eloraam
Copy link
Contributor Author

Eloraam commented Jul 2, 2020

My question is whether, specifically, "Cap'n Pester" should attack the player on sight. He tends to be a bit out of scale with the locals in most parts of the galaxy.

@tehhowch
Copy link
Collaborator

tehhowch commented Jul 2, 2020

My question is whether, specifically, "Cap'n Pester" should attack the player on sight. He tends to be a bit out of scale with the locals in most parts of the galaxy.

That's certainly true, but is consistent with the reference.

What could be done, and I wouldn't be opposed to, would be to make the Parrot government only enforce within non-human systems, via an enforces node. That will unfortunately also prevent normal fines from being levied for e.g. Nerve Gas.

@tehhowch tehhowch added balance A ship or weapon that seems too powerful or useless, or a mission that seems too easy or hard mechanics Things dealing with the mechanics & code of how the game works and removed unlikely An enhancement or other change that is lowest priority labels Jul 2, 2020
Now only restricts Parrot and Author to enforcing in Human and Hai
space.
@Eloraam
Copy link
Contributor Author

Eloraam commented Jul 2, 2020

That was difficult to test. Had to temporarily patch the source code to make them appear a lot more often to get it working.

I figured that they should probably enforce in Hai space too.

@Eloraam Eloraam changed the title Parrot and Author governments no longer give fines. Parrot and Author governments enforce only Human and Hai systems. Jul 2, 2020
@tehhowch
Copy link
Collaborator

tehhowch commented Jul 2, 2020

An alternate solution would be a bit more expansive, but preserve the current fining behavior.
It would need

  1. custom atrocity penalties for Author & Parrot of 0 by default
  2. code changes
    1. scanning methods now return a fine amount, and also whether or not an atrocity item was found, e.g. pair<int64, bool>
    2. if a government doesn't care about atrocities (PenaltyFor(scan) == 0), then scanning the player could only levy a fine rather than returning a message sentencing the player to death. (I'm not sure whether we should still call Offend to notify other governments of the atrocity outfit discovery, discussion would be needed.)
  3. a follow-up mission-triggered event that restores the current atrocity penalties after some condition (like completing the main plot or discovering some region of space or making some other government hostile).

The bits from 2) are probably viable in their own pull, as the atrocity mechanic is not exactly used often and the current behavior seems not-quite-right, for governments that scan the player but don't care about atrocities.

@Eloraam
Copy link
Contributor Author

Eloraam commented Jul 2, 2020

So if I understand you, the idea is that after the Free Worlds questline, the current state should be restored, and you should be attacked by Pester in say... Korath space?

If that's the case, I'll have to rethink the content I'm developing (plugin for now, but trying to keep it stock-like).

Eloraam added 5 commits July 2, 2020 23:04
Implemented a new 'legality' node that allows individual governments to
treat specific outfits or missions differently.  No change to stock
gameplay.
@Eloraam Eloraam changed the title Parrot and Author governments enforce only Human and Hai systems. Fine-grained Illegal/Atrocity Control (was Parrot and Author governments enforce only Human and Hai systems.) Jul 3, 2020
@Eloraam
Copy link
Contributor Author

Eloraam commented Jul 3, 2020

Pure mechanism now, no policy at all. No change to existing gameplay of stock or plugins that don't use it. And a nice new feature.

And it still lets me solve the problem I'm having in my plugin.

@BaddestApple
Copy link

I like the idea of being able to set legality and fines differently depending on the government, but would you be able to provide some clarification of the syntax?

In the example legality block, the named legality would be an atrocity in the Republic and a fine of 20,000 in the Free Worlds, but what does the default illegal xxx and default atrocity line do? If it's to set a different legality level for all other governments apart from the named ones, wouldn't default illegal xxx and default atrocity be mutually exclusive?

And how I do integrate the legality block into an outfit? "Illegal name" and "Atrocity name" is a little unclear; do I replace illegal xxx with illegal "name" in the outfit, with the "name" being the legality block?

For example, is this syntax proper or am I butchering it?

outfit "Smuggling Compartments"
	category "Tactical"
	cost 75000
	thumbnail "outfit/smuggling compartments"
	"mass" 3
	"cargo space" 5
	"weapon capacity" -2
	"outfit space" -2
	"engine capacity" -2
	"cooling inefficiency" .5
	"heat dissipation" -.02
	"scan interference" 4
	illegal "smuggling"
	description "	The classic Smuggling Compartments find hidden nooks and crannies anywhere they can between your outfit, weapon, and engine spaces, giving additional smuggling capacity and scan interference with its specially designed scan interference lining. Hiding cargo in your bulkheads requires sacrificing some cooling efficiency and heat dissipation, however."
	description "It's illegal to outfit your ship in this way... that is, if scanners or inspectors are able to discover your hidden stash."

legality "smuggling"
	default illegal 1000
	illegal 500000
		government "Republic"
	illegal 20000
		government "Free Worlds"

@Eloraam
Copy link
Contributor Author

Eloraam commented May 24, 2021

@BaddestApple Yes that's correct. Looks like you have it.

And yes, the two defaults are mutually exclusive, exactly as you think. I was just listing all the recognized syntax, not providing an example.

@Zitchas
Copy link
Member

Zitchas commented May 24, 2021

Anyway, I really like the idea of being able to establish different legalities for outfits.

Could this be used to establish variable legality, not just between something being an atrocity versus being merely illegal, but also allow something to be legal?

For example, the Remnant overall have an ambivalent attitude towards nerve gas. In their view, dead is dead: Killing someone by shooting them is just as bad as killing them with poison or blowing up their ship. There's nothing "good" or "noble" about it, it is just something unpleasant that sometimes needs to happen. Soldiers are expected to use the most appropriate tool for the job, whatever that happens to be.

So they wouldn't care if someone was carrying nerve-gas... Unless the person wasn't friendly with the Remnant, in which case they'd be likely to "kill on sight" anyone with Nerve Gas.

@Eloraam
Copy link
Contributor Author

Eloraam commented May 25, 2021

@Zitchas I'm pretty sure that if you set the fine to 0, by saying "default illegal 0" or "illegal 0", that the outfit or cargo is considered legal either by default, or by that government. I used it that way in my own test case.

It wouldn't be very difficult to add the name "legal" to those cases, if anyone's still interested in this patch.

Copy link
Collaborator

@tehhowch tehhowch left a comment

Choose a reason for hiding this comment

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

Needs conflict resolution at a bare minimum; style fixes would be appreciated too.

w.r.t completeness:

  • I'd like to see ideas / mockups for displaying variable legal status for outfits which are only illegal to certain governments, which would be obscured from the player based on the current PR.
  • Should legality definitions be extendable via events?
  • How should the game handle plugins defining the same legality node? As I read it, due to use of map::emplace, the first value always wins, which is the opposite behavior of most of the game's data models.

#ifndef LEGALITY_H_
#define LEGALITY_H_

#include "DataNode.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be a forward declaration, with the full include in the TU.

Comment on lines +160 to +161
// Duplicated for outfitter display.
attributes[child.Token(0)] = legality->GetFine(nullptr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like a better place for this is in OutfitInfoDisplay, where we could then show additional details about the illegality of the given item in this or neighboring systems, or some other manner, that enables the player to know that some government / planet they are aware of (and probably only those they are friendly with / could land on) disapprove of the given outfit.

There definitely exists an argument for deferring that implementation of viewing the additional details to a follow-up PR, but a counterargument is that the current outfit display system does not hide information about an outfit being legal (or not), while with this system it could.

Comment on lines 55 to 57
#include "System.h"
#include "Legality.h"

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a number of style nitpicks in the patch, e.g. unalphabetized includes, syntax like T* identifier instead of T *identifier, and non-spaced operators (e.g. var=-1 instead of the desired var = -1).

@@ -16,6 +16,7 @@ PARTICULAR PURPOSE. See the GNU General Public License for more details.
#include "Weapon.h"

#include "Dictionary.h"
#include "Legality.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be a forward declaration rather than an include, since this class only holds a pointer to the type

}
else if(child.Token(0) == "atrocity")
{
if(child.IsNumber(1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not your fault, but the class method really looks incorrect... "of course 1 is a number..."

To avoid calling child.Token(1) when there is just "atrocity":

Suggested change
if(child.IsNumber(1))
if(child.Size() < 2 || DataNode::IsNumber(child.Token(1))

@Zitchas
Copy link
Member

Zitchas commented May 25, 2021

I'd say "Yes, definitely" to the "should they be extensible via events?" question.

Honestly, the more that events can change, the better. Especially for something like legalities. Heh. It'd actually be kind of interesting if "what is legal" in the Republic change a bit depending on how campaigns end. If you end up on the checkmate ending, then, say, certain illegal outfits aren't fined as heavily. (or maybe certain atrocities aren't are merely heavily fined)

@Galaucus
Copy link
Contributor

There's been some discussion on the Discord channel regarding whether to assign illegality at the government side of things or in the outfit itself.

General consensus - and the approach I support - is behind doing it on the government panel. This prevents the problem of spoiling the existence of governments that the player hasn't met, and we can move any player-facing information to the logbook by creating an entry (and corresponding popup) any time they encounter an illegal outfit being sold.

We'll also be instituting a new hailing panel that pops up when ships scan you, and it's cleanest to just do that all from the same place; the government definition.

It's my hope that we can implement such a change as part of a more comprehensive smuggling overhaul as outlined in #6314 .

@Hurleveur
Copy link
Member

I just noticed that one of the PRs I was asked to do is a duplicate of this (#6407)
I'll try to see if mine matches what tehhowch asked to change here soon, since the owner of this PR seems away.

@Eloraam
Copy link
Contributor Author

Eloraam commented Jul 14, 2022

Vaguely paying attention but really not having the time to update the conflicts. Have at it.

@Hurleveur
Copy link
Member

Hurleveur commented Jul 14, 2022

We really did it in different ways. Too different for me to be able to implement any of this really.
For instance, mine it's only definable from the government's perspective.
But if someone disagrees I could make some changes to mine.

I don't see any change tehhowch asked that I can actually apply to my PR.
Also mine allows for making an ex illegal outfit be ignored by a given government, and for all of it to change by events.

Vaguely paying attention but really not having the time to update the conflicts. Have at it.

As far as I can see its been a year this is open and you haven't found the time to address tehhowch's requests.
Do you think you will be able to find some time in the future, or should this PR be closed?

@Eloraam
Copy link
Contributor Author

Eloraam commented Jul 14, 2022

I could probably find the time to clear the conflicts, but some of the requests looked a lot more involved than that. I'm not currently developing a plugin anymore, so a deep dive isn't likely at the moment.

Your PR would have solved my problem anyway.

@Hurleveur Hurleveur added the duplicate Commentary and updates should be added to the original, and not this label Aug 1, 2022
@Hurleveur
Copy link
Member

Hurleveur commented Aug 1, 2022

In that case I'll be closing this PR since we prefer to only have active PRs open.
It is not permanent by any mean, feel free to reopen it when/if you feel like working on it more!
The thing is that, on top of the inactivity, this is a duplicate.

@Hurleveur Hurleveur closed this Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
balance A ship or weapon that seems too powerful or useless, or a mission that seems too easy or hard duplicate Commentary and updates should be added to the original, and not this mechanics Things dealing with the mechanics & code of how the game works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants