Feat(mechanics) : custom outfit prices and sell type

[Hurleveur]

Feature: Outfits can have custom prices depending on what planet they are sold on, and be imported.

Feature Details

All outfits can now have varying prices and sell types, with the highest price taking priority and import > default when there are conflicts. Outfitter price will also be ignored when there is an outfit price specified. The map outfitter panel also has varying colors depending on the prices, same for the map shipyard panel and there is a legend ! You can define both an offset and a base price, both always being interpreted as pourcentages relative to the base price. So, if an outfit costs 1000 and you write 10 that'll be interpreted as 1% of the base cost if its a value, and 1% on top of the modified cost if it is an offset.

This also support event removal and adding. If you do not specify add before value or offset the previous offsets/values will be replaced by the new ones. For removal it means that if you just write remove it will disable the entire customSale (destroy it), remove outfit "Jump Drive" will remove that one from all vectors whereas just remove outfit will empty all outfits from the vectors.

UI Screenshots

I removed the legend changes from this PR, it will just show if the item is there, like before. (used to be:)

This is an example with prices going from 1000 to 20000 with the laser rifle. note : I've changed the legend to be even more representative, now the second number shown in the legend is right between the first and the third. The prices for ship vary too, depending on where they are landed/sold (when not on a planet it is based on default outfit price).

Usage Examples

Works within events

pricing <outfits/outfitters> <identifier>
  [import / normal (implicit if you do not write anything)]
  location <planet/ (nothing if you use a filter below) >
    [{... LocationFilter specification}]
  conditions
    [{... ConditionSet specification}]

  [offset / value (implicit if you do not write anything)]
    "outfit name" <number> (optional: write '%' if the value before is to be interpreted as a modifier to base value instead of a new price)
  if needed: [offset / value (different than what was just used)]

Import means you can only sell them, except that you will still see it in the outfitter with an "(import)" notice.

The new prices will override the base ones, and if multiple prices are specified for one place the highest one will be taken normally, except if one of the two is hidden/import that take priority respectively over import and base/base.

Concrete examples

Concretely, say you want the broken jump drive in remnant space to sell (and not be buyable) at 1000 000, and only once a given mission was completed; here is what you should write:

pricing outfits "remnant broken jump drives"
  import
  location
    government "Remnant"
  conditions
    has "Remnant Bounty Hunter: completed"
  
  value
    "Jump Drive (Broken)" 1000000

Now about the offset, it cumulates with the value, meaning you can easily have a special event that adds 10% of the new price, whatever that price may be at the same:

pricing outfits "remnant broken jump drives"
  import
  location
    government "Remnant"
  conditions
    has "Remnant Bounty Hunter: completed"
  
  offset
    "Jump Drive (Broken)" 0.1 %

This, on top of the other jump drive modifier, makes it cost 1 100 000

Testing Done

Loaded a game and changed the prices with imports, hidden, value, offset, relative or not and for outfits as well as for outfitter.

Performance Impact

Not very significant, the extra operations are checking the price of outfits and ships (obviously) but it only happens when the game is in pause, aka when in the map outfitter/shipyard panel or in the outfitter. The new class, CustomSale, uses different std:maps because there is a need for independence of offsets and plain value (relevant when doing sales, which is applied on top of the base value but is in a different pricing) and outfits and outfitter are separated to be able to differentiate them in case of a conflict. Also it would not make sense to make a pricing for which the player gave an outfit depend on a Sale<Outfit>, and on top of that we would have to make it individual Sale<Outfit> for each element, given that their specified price differs.

[tehhowch]

After thinking a bit further, the only reason for the visibility modifiers in what you've done here is because you were hacking on top of how we specify the things that are sold.

Removing that hack and using a new class for the express purpose of changing the price of items at matched planets means none of the import / hidden stuff is even necessary.

[Hurleveur]

I don't agree with changing outfitter (which is just a grouping of outfits and is not planet specific) with cost information, as the likely desired use of this new functionality is planet-specific costs and availability.

I feel like what you are proposing is a niche thing, the idea behind this system is for it to be easy to for example make the prices of lasers in all of republic space go up in one line, so it offers way more possibilities for the content creator, whilst also allowing for one planet with specific prices. So even if you want to propose something that still decodes for an entire kind of outfitters, that the game would have to decrypt on launch (time lost) there will be a lot of redundancy in information contained by the game, thus memory used, like for a concrete example : every single Hai planet buys remnant keystones at 12000 tldr unless if I understood smth wrong what you propose is less optimised and more space heavy

You should instead introduce a new class that handles this responsibility. It should be able to be applied to one planet or more planets easily, and it should be modifiable by events. It probably does not need to be serialized.

I think my class is already modifiable by events, just add outfits on top of it if they have priority else remove and re-add from outfitter

An example class that controls pricing via a flat scaling on a per-list basis, i.e. to make everything in the list cost x% of the base price. (One could conceivably make it per-item and support being fed a Sale<Type> as well as a single item.)

I do agree with containing a % instead of a price, because often it is actually what I need !

Also, the legend colors in the screenshot are not ok. The same range of colors should be used consistently to represent prices, i.e. the minimum price should always be the same color and the maximum price (or prices above a reasonable limit) should always be the same color.

that is totally the idea behind the way the colour scheme works, that is why I have max and mincolor be dynamic and then multiply each relative price by it, so top price is always pink and bottom is always dark green. Normally it is the case already I don't use the default color scheming caus it's bad

[Hurleveur]

After thinking a bit further, the only reason for the visibility modifiers in what you've done here is because you were hacking on top of how we specify the things that are sold.

Removing that hack and using a new class for the express purpose of changing the price of items at matched planets means none of the import / hidden stuff is even necessary.

I don't quite follow sorry how will defining a custom price prevent the outfit from being sold somewhere ? or only be shown if you have one ?

[tehhowch]

I'm guessing I didn't explain myself well; my apologies.

In the architecture I propose, the content creator (CC) is able to declare price modifications that apply at matched planets, for items that match their criteria.

1. The CC is able to specify the planet(s) using standard LocationFilter syntax, in exactly the same manner as one would specify a mission's source, destination, or stopover.
2. The CC is able to specify a price modification as some combination of flat and/or proportional amount.
3. The CC is able to specify the items that are affected by the price modification in either or both of two manners:
   • they may reference one or more game-managed lists of items by name.
   • they may reference one or more specific items by name
4. The selected syntax for the definition may be such that the CC can group multiple price-item pairings per planet criteria
5. When the player goes to purchase or sell an item, the list of items for sale remains no different than it is today: the items listed in the planetary stock (Sale<Outfit> or Sale<Ship>), or that they own and have present. The list of price modifications that apply to the current planet is fetched, and each is evaluated to determine the best price for the selected item.

With this approach, the planet never needs to know what discounts or surcharges are imposed, so the "outfitter" syntax needs no modifications. All the item references are pointers to GameData content, so there isn't much space overhead, either--just the collection of discounts and the location filter.

Hopefully that's a little clearer than the toy class I threw together earlier.

[Galaucus]

Here's the intended use-case, if that's of any help.

outfitter "Smuggling: Smuggler's Den"
	"Chemical Warhead"
	import
		"Coreshine" 11500
		"Stolen Ship Parts" 89000
	
outfitter "Smuggling: Copperline Station"
	"Coreshine"
	import
		"Stolen Ship Parts" 125800
		"Liberator Missile System" 17900

outfitter "Smuggling: Skymoot"
	"Dragon's Teeth"
	import
		"Liberator Missile System" 43500
		"Designer Drugs" 12000

outfitter "Smuggling: Glory"
	"Designer Drugs"
	import
		"Dragon's Teeth" 90188
		"Jailbroken AI Core" 160711
	
outfitter "Smuggling: Sleezy Ed's"
	"Stolen Ship Parts"
	import
		"Counterfeit Credsticks" 150
		"Coreshine" 6400

outfitter "Smuggling: Haven"
	"Liberator Missile System"
	import
		"Chemical Warhead" 1137000
		"Dragon's Teeth" 98250

outfitter "Smuggling: Warehouse 0x16A"
	"Counterfeit Credsticks"
	import
		"Jailbroken AI Core" 188711
		"Chemical Warhead" 927400

outfitter "Smuggling: Bivrost"
	"Jailbroken AI Core"
	import
		"Designer Drugs" 11635
		"Counterfeit Credsticks" 180
	
outfitter "Smuggling: Mars"
	import
		"Chemical Warhead" 873000
		"Coreshine" 3900
		"Dragon's Teeth" 89000
		"Designer Drugs" 11415
		"Stolen Ship Parts" 53800
		"Liberator Missile System" 15500
		"Counterfeit Credsticks" 115
		"Jailbroken AI Core" 157711

outfitter "Smuggling: Trinket"
	import
		"Chemical Warhead" 852200
		"Coreshine" 6700
		"Dragon's Teeth" 87750
		"Designer Drugs" 11795
		"Stolen Ship Parts" 53800
		"Liberator Missile System" 25500
		"Counterfeit Credsticks" 145
		"Jailbroken AI Core" 172711

[Galaucus]

I don't believe specifying these using location filters would be wise. For the smuggling system, each planet needs its own outfitter anyways to get them to stock their planet-specific export; being able to define what they import in the very same place saves a lot of trouble.

Furthermore, we expect to use the hidden attribute in all Remnant outfitters to allow them to buy Hai keystones at a higher price. It's a much simpler manner to assign that at the outfitter level than to every planet which happens to have a Remnant outfitter.

[Zireael07]

@Galaucus: Nit: "sleezy" should be "sleazy"

[Galaucus]

@Galaucus: Nit: "sleezy" should be "sleazy"

It's intentional.

[Zireael07]

Needs a translator comment in that case.

[Galaucus]

It'll be explained in the upcoming missions featuring Ed.

[tehhowch]

I don't believe specifying these using location filters would be wise. For the smuggling system, each planet needs its own outfitter anyways to get them to stock their planet-specific export; being able to define what they import in the very same place saves a lot of trouble.

I'm not particularly concerned with creating the tersest way for a content creator to achieve a single task. I would much rather we provide a flexible new tool that is widely applicable that also achieves the desired task.

outfitter "Smuggling: Trinket"
	import
		"Chemical Warhead" 852200
		"Coreshine" 6700
		"Dragon's Teeth" 87750
		"Designer Drugs" 11795
		"Stolen Ship Parts" 53800
		"Liberator Missile System" 25500
		"Counterfeit Credsticks" 145
		"Jailbroken AI Core" 172711

Is not much easier than

pricing outfits <identifier>
  location [<planet>]
    [{... LocationFilter specification}]
  items
    <item name>
      [delta <#flat-amt>]
      [percentage <#amt>]
      [value <#>] # always sells at this amount even if other deltas or percentages could apply
    ...
  "item groups"
    <item group name>
      {... same pricing modifiers}
    ...

The keywords there are obviously not set in stone. With a dedicated pricing class, you could even simplify your approach - the items go into a single outfitter list, and your planet-specific pricing definitions would only need to override a few base prices. You could even regionally adjust a subset of other prices without needing to look at a dozen outfitters and a spreadsheet to compare prices.

Furthermore, we expect to use the hidden attribute in all Remnant outfitters to allow them to buy Hai keystones at a higher price. It's a much simpler manner to assign that at the outfitter level than to every planet which happens to have a Remnant outfitter.

As an example:

pricing outfits "remnant adjustments"
  location
    government "Remnant"
  item
    "Quantum Keystone"
      value 125000
  "item groups"
    "Deep Weapons" # or some other advanced tech outfitter
      percent 125

The benefit over hacking outfitter syntax is that new extensions do not suffer any backwards compatibility issues at all. outfitter is extremely simple--it's single responsibility is to be a collection of items. This PR removes "collections of outfits" and forces all of existing places that just need a collection of outfits to be "collections of outfits with prices and visibility."

(As an example, it is trivially easy to extend this "pricing" class with other things relevant to pricing - a specific icon or caption that should be rendered when the price applies, a condition that sounds be set after purchasing with the discount/markup, etc. None of that is relevant to a syntax whose purpose is to group a collection of items.)

[Hurleveur]

I have to agree with you on the principles but this means I need to restart everything from scratch and it's already taken me a few weeks to do it properly with all the tests I did... Next time I'll ask you how to do smth like this, I asked quyykk and amazinite if my way of doing it was fine and it didn't seem like there was going to be a problem...

anyway I want to be sure to get it right this time : this class is meant to be contained within each planet ? I have no idea how to do what you ask (because of the many complicated implications + performance issues I already see with your approach), I feel like you are seeing too far ahead and asking a bit too much, hopefully I'll manage otherwise I hope smbd else will do this

[Amazinite]

I have to agree with you on the principles but this means I need to restart everything from scratch and it's already taken me a few weeks to do it properly with all the tests I did...

Such is the case with software development sometimes. The first approach taken isn't always the best and you have to toss out what you originally had.

Next time I'll ask you how to do smth like this

The better approach is to open an issue describing what exactly you want to do (in this case allow planets to buy or sell outfits are different prices than the standard) then provide an example of how you think it could be done, either in syntax or implementation details (in this case through adding additional tokens to outfitter). Then others can chime in on the issue describing any additional or similar features they might want handled or with a better implementation details.

Either that or approach your first implementation as a prototype with the expectation that it won't be the best. Prototypes should be quick and dirty; sometimes a prototype is good enough to get the idea across and might in the short term be exactly what you want, but could have issues with expandability or compatibility. A prototype at least gives you a sense of if your idea will work out, and you can use what you learn from the prototype to make something better in the end. And even if the idea never gets finished, at least you got more experience with the codebase.

A good example of this is how I approached system hazards; I first posted the issue #4116, which linked to various other places where the concept had been discussed before. There was then a good amount of discussion on what people might want from system hazards, and some people provided example syntax for how hazards might be defined and design details as to how they think such a system might be implemented. We then discussed the merits of the potential implementations described, such as whether hazards should basically just be special ships akin to the nanobots (and the answer was no). After all that is when I first opened #4931, which was initially only a prototype showing how hazards might be done. The prototype was met with further discussion on how it falls short and what more it could do and how hazards could be better implemented. Then I took the ideas from the prototype and from the resulting discussion and completely redid what I had. So while the original work was tossed out and I almost entirely restarted from scratch with it, that was the expectation from the start and it was for the better that I did so.

The time scale can certainly be shorter than it was for system hazards (sometimes different steps run together because the feature is simple and you can get through them quickly), but the general idea is there for how you should go about developing a new feature.

TL;DR I felt like making this a teaching moment for some reason. Basically just don't feel bad if you have to restart from scratch because something of value can still be gained from the work you did and the end product will improve because of it.

I asked quyykk and amazinite if my way of doing it was fine and it didn't seem like there was going to be a problem...

Well I did only give it a shrug (¯\_(ツ)_/¯), as I hadn't looked into the idea too deeply. There wasn't anything that jumped out at me from the original implementation details as being too bad, but sometimes you only learn the shortcomings of your approach after you've taken it.

[Galaucus]

Part of this is on me for asking for such substantive changes without documenting it properly in #6314 . I'll definitely be more thorough next time.

[Hurleveur]

actually it's not that bad I can keep the same principle I'm using now, just need to add it on top of Sale<Outfit> within planet, and have my class only contain %, whilst retaining the container linked to the outfit pointers in the map. Then I still don't know how to read smth new from a file but that fine, I may have been overestimating the complexity of it

[Galaucus]

@tehhowch We're very happy to to defer to you if you can think of a good syntax for tracking this on the data side of things. Got anything in mind?

[Hurleveur]

there is a new feature that would be nice but I am really not sure how to implement correctly (I know how to hack it though, as always x) ) : having this work like it does with missions with conditions. I already have the code to do it I just didn't upload it because I'm not sure you would accept it, what I did is take the player's conditions and manually change the flagship's system and planet to match the planet that is concerned with the local cost (this is relevant for when you look at the outfitter map, it needs to show the right prices even when you're not there). Then we just compare with the conditions given. I feel like leaving I should this open for discussion how it should best be done instead of like before just doing it my way, the conditions could be made to exclude planet and system because we are already considering them in location now that I think about it...

Anyway adding a "condition" field to the pricing could always be done in another pr after this one.

edit: I did it! works with conditions

[Hurleveur]

smbd just noticed some issues because of the fact this system is over the normal outfitter system, will take a bit to solve without completely murdering optimization (but it'll get hit, as planned)

[Galaucus]

Anyway adding a "condition" field to the pricing could always be done in another pr after this one.

Yeah, let's leave that for later. I'd rather have this functional and ready to support the smuggling system as a first priority - we can always expand on it later, yeah? My priority as the content creator for the smuggling overhaul is for this to simply support it, and do that well.

As I understand we can always upgrade and expand that system later when there's anyone wanting to make price adjustments beyond the smuggling stuff, but for now I'd be happy just to have it working.

[Hurleveur]

I think this is pretty much done, it works with events, conditions, locations, outfitters, outfits, with value, offset and %. And it even has a legend to come with it, with all changes showing on the map outfitter, as well as shipyard, and ofc in regular shipyard and outfitter with the prices being different. In short, I think I'm finally done! And it should correspond to everything that was asked in terms of coding and formatting.

[Zitchas]

I'd like to make a request: Could you add an example to the first post showing some utilization of this?

For example, making Broken Jump Drives cost 1000000 in the Remnant outfitter.

edit: there appear to be a bunch of things breaking here. Hopefully not too serious?

[Hurleveur]

Okay I will show such an example, after fixing the errors, shoudn't take that long. No worries I just merged it with master incorrectly, should be easy to fix, its just tiresome to always have to keep this up to date whenever there is a conflict.

edit: was just my dislexia getting what's master and what's my branch confused in the merge, now I'm just going to have to pray a bit that I wont screw git up

[Zitchas]

Just to clarify, I was hoping for example code. (I know there's various examples in the conversation, but now that this is basically done, having an up-to-date current example would be nice.)

[Zitchas]

Thanks for the Broken Jump Drive example. That's exactly the sort of thing I was hoping for.

[Hurleveur]

Thanks you so much for that review, I always learn new things and it keeps me motivated to work on this project! A few things you point out are sometimes obscure to me but I do agree with what you point out, so much more to learn still...

edit: (Also, I recommend avoiding the explicit use of "outfit" and "outfitter" in the user-facing syntax for defining pricing overrides. Prefer class-agnostic identifiers so that future expansion of this system will not require keyword changes.) What do you mean by that? Make the syntax force the user to repeat "outfit" for each outfit?

[tehhowch]

Also, I recommend avoiding the explicit use of "outfit" and "outfitter" in the user-facing syntax for defining pricing overrides. Prefer class-agnostic identifiers so that future expansion of this system will not require keyword changes.

What do you mean by that? Make the syntax force the user to repeat "outfit" for each outfit?

I'm referring to the user-facing syntax for a custom sale using "outfit" and "outfitter" to denote DataNodes that refer to the single and multi unit entities. That prevents us from reusing the implementation for a different kind of entity without also changing how the class parses text. We can instead encode the entity type context into the root DataNode that introduces the custom sale, e.g. pricing <type> <identifier>.

[Hurleveur]

Unfortunately as of now I cannot make the tests work (or it would be hacky) and keep this PR code change only, as data cannot be temporarily loaded into the tests, so any test has to be made based on the actual game data. I could for instance add a condition "is testing" to activate specific pricing for testing but it would be hacky, and even then, there is no way to guarantee what item will be the first one in the list (which is needed for reliable testing with keyboard input only), as there is no shortcut to go into sell from cargo with keyboard input. What could also solve this is a search function in the outfitter.

But as of now, there is no way to make automated tests for this PR, however I will do them in a follow up PR as soon as possible.

[Hurleveur]

I don't know how I did not think of this, but events can contain custom prices, and said events can be located in savefiles. So I'll write these tests shortly, adding an outfitter to a desolated planet to make sure to control what is there, and what isn't.

[Hurleveur]

Also I would just like to point out if anyone wants me to, I can easily expand this feature in a follow up PR. By that I mean adding the possibility to have custom shipyard prices as well, which wouldn't be hard to do now that all of this was established.

[Zitchas]

Having this apply to ships too would be rather nice; but I agree that's probably best for another PR.