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 support for Program Modifiers #2423

Merged
merged 1 commit into from
Aug 18, 2024
Merged

Add support for Program Modifiers #2423

merged 1 commit into from
Aug 18, 2024

Conversation

siimav
Copy link
Contributor

@siimav siimav commented Aug 15, 2024

These will change the funding, duration etc values on a program if another gets accepted.

@siimav siimav changed the title Add support for Program Modifiers. Add support for Program Modifiers Aug 15, 2024
Copy link

github-actions bot commented Aug 15, 2024

Download the artifacts for this pull request:

{
if (ProgramHandler.Instance.IsProgramActiveOrCompleted(pm.srcProgram))
{
pm.Apply(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

What if you define two PM config with different source programs that modifies the same target program, you'd apply both and I am not sure the result would be deterministic here or be the intended effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PMs get applied in the order they are configured and thus the effects should be deterministic.

Copy link
Contributor

Choose a reason for hiding this comment

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

So since "modifiers" just a new value configuration, the last one configured is the only one used at the end right? or could we do partial PM configs that could be combined? (One defines nee confidence, one defines new fundingcurve)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They would combine.

@Karhgath
Copy link
Contributor

Looking at the code, it might be possible to do an improvement where you could define a comma-separated list of srcProgram and have a AreActiveOrCompleted function instead. If would be considered a AND. It could enable scenarios like boosting both Early Sat programs if you have both Rocket Dev and Suborbital Research active or completed.

@siimav
Copy link
Contributor Author

siimav commented Aug 16, 2024

Looking at the code, it might be possible to do an improvement where you could define a comma-separated list of srcProgram and have a AreActiveOrCompleted function instead. If would be considered a AND. It could enable scenarios like boosting both Early Sat programs if you have both Rocket Dev and Suborbital Research active or completed.

Honestly that's not what I'm interested in right now. This should be tackled separately once we decide that such a feature is needed.

These will change the funding, duration etc values on a program if another gets accepted.
@siimav siimav merged commit a6ea8bf into master Aug 18, 2024
3 checks passed
@siimav siimav deleted the PMs branch August 18, 2024 20:43
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.

2 participants