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

Fix/ Improvement for NoCargoBountyArbitrageTest #34883

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GansuLalan
Copy link
Contributor

@GansuLalan GansuLalan commented Feb 4, 2025

About the PR

Fix for the cargo arbitrage test regarding bounties as it was not covering many cases

Why / Balance

Was reading other PRs and saw in #34335 mention of a bounty arbitrage integration test that, considering the prevalence of flipping order for bounties right now, wasn't working. This is an attempt to rectify this though there are concerns regarding runtime as the number of permutations is quite a bit. I don't think it currently covers restock breaking that you can do for the donut bounty and some other methods involving multiple orders.

Technical details

The current code works by spawning in a single box of each product and checking if it fulfils any bounties while costing less, while this catches some cases it fails to account for the idea that you can simply order 2 of a product and flip it for the bounty. This code works in a similar way except it will check with multiple boxes of a product and will skip any irrelevant bounties as a time saving measure. This has increased the run time for the test substantially but not overwhelmingly and helps to spot more cases, there are a lot of cases...

There is probably further room for optimisation if needed and I can continue work if this is desired but the current speed should be sufficient for a PR I hope...

My favourite discovery is that you can fulfil the laser bounty worth 28500 by ordering two firefighter boxes for 3000 as batteries count for this bounty as it lacks a blacklist...

Media

Requirements

Breaking changes

None that I am aware of, only potential thing is making a method public since I didn't want to do a copy paste but I fail to see how this would ever cause a problem

Changelog

No changelog as it is test related

@github-actions github-actions bot added size/S Denotes a PR that changes 10-99 lines. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Feb 4, 2025
@GansuLalan
Copy link
Contributor Author

Would fixing any integration test fails also need to be a part of this sort of PR to prevent mass failings going forward? I can do that but those sorts of decisions are scary to make so I'd prefer some guidance

@Plykiya
Copy link

Plykiya commented Feb 4, 2025

yeah I ran into that same kind of silly test fail in #34851 regarding plastic knives

@GansuLalan
Copy link
Contributor Author

GansuLalan commented Feb 4, 2025

Yes, currently fails but it does miss some other bounties I know you can do with a digipad without leaving the ats:
Crayons with art supplies
Donut with donut restock
Figure with mystery figures
handcuffs with cable crate
knife with glass and cloth crate

Some others as well probably, there are also orders you can match with extra, ones like the monkey cubes, you can order a crate, take out the extras, sell for the bounty and then sell the spare for a profit of 185, though you can also go even further if the organ bounties are there as well.

These cases can probably be accounted for as well but the joy of the cargo bounty arbitrage is that there's quite a few ways to game it.

@Tayrtahn
Copy link
Member

Tayrtahn commented Feb 4, 2025

Would fixing any integration test fails also need to be a part of this sort of PR to prevent mass failings going forward? I can do that but those sorts of decisions are scary to make so I'd prefer some guidance

They need to be fixed before this PR can be merged. I would probably do it in a separate PR and reference this one, but you could do it as part of this one if you'd prefer.

@GansuLalan
Copy link
Contributor Author

Is there anywhere you'd suggest discussing this, I can fix some of these by updating a blacklist or do a slight price shift, but some of the changes are quite big as well, like doubling a requirement/ halfling a reward, especially since I think it's best to avoid touching the products as most of them feel quite good

@ArtisticRoomba ArtisticRoomba added P3: Standard Priority: Default priority for repository items. T: New Feature Type: New feature or content, or extending existing content D3: Low Difficulty: Some codebase knowledge required. S: Needs Review Status: Requires additional reviews before being fully accepted A: Integration Tests Area: Integration tests, adding or fixing them and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Feb 5, 2025
@ArtisticRoomba
Copy link
Contributor

My favourite discovery is that you can fulfil the laser bounty worth 28500 by ordering two firefighter boxes for 3000 as batteries count for this bounty as it lacks a blacklist...

you can do what???

@deltanedas
Copy link
Contributor

batteries are battery ammo providers so you can spend 6 steel and plastic for it

@ArtisticRoomba
Copy link
Contributor

incredible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: Integration Tests Area: Integration tests, adding or fixing them D3: Low Difficulty: Some codebase knowledge required. P3: Standard Priority: Default priority for repository items. S: Needs Review Status: Requires additional reviews before being fully accepted size/S Denotes a PR that changes 10-99 lines. T: New Feature Type: New feature or content, or extending existing content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants