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

Add scripting support for cost expenditures (requires #21932) #22039

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

ltsSmitty
Copy link
Contributor

@ltsSmitty ltsSmitty commented May 15, 2024

TODO:

  • why is it failing on x64 but not arm?
  • add native window in the scenario editor with the same functionality

This PR adds on scripting functionality to #21932.

A few comments:

  • I've opted to put the getExpenditureMultiplier and setExpenditureMultiplier functions inside of the Park interface. Potentially this could go inside of Scenario instead, but that seems more closely related with the objective needed to win the park.
  • I've also created a ExpenditureTypeEnum enum in the .d.ts instead of a number for the functions. It's duplicative with the text of the existing ExpenditureType, but serves a different purpose. If it decided that this makes the .d.ts file too bloated, another option is to add these number as comments into the ExpenditureType so developers have something to reference.
  • The current build uses a percentage-point system to represent multiples (100=100%, 50=50%), but that could be changed to using a dime system like ride costs (1.2 = 120%, 2.5 = 250%). Some feedback was given about which to use, but this could also be transformed on the frontend as well.

I'd appreciate any testing to ensure this works as expected. You can download the built version of this on many systems via the built tests (though some failed and I'm not sure why), and I've attached a plugin you can use to test the main functionality.

@Basssiiie Basssiiie requested review from duncanspumpkin and Basssiiie and removed request for duncanspumpkin May 28, 2024 17:28
@Basssiiie
Copy link
Member

Basssiiie commented Jun 1, 2024

Regarding ExpenditureTypeEnum, we should not have duplicate enums for the same thing, I do think it's bloat and may reduce compatibility between various properties and methods. You mention it serves a different purpose, but what purpose? It looks the same? 😅

For the rest I think I'll await the discussion of #21932 first.

@Sadret
Copy link
Contributor

Sadret commented Jun 1, 2024

Also, why don't your functions just use the already-existing ExpenditureType? I.e. let the functions have string args and not number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants