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

Backwards Compatibility Policy #31

Open
Iridar opened this issue May 19, 2021 · 3 comments
Open

Backwards Compatibility Policy #31

Iridar opened this issue May 19, 2021 · 3 comments

Comments

@Iridar
Copy link

Iridar commented May 19, 2021

Backwards Compatibility Policy

The Community Promotion Screen is intended to be backwards compatible with New Promotion Screen By Default as much as possible, as long as it doesn't interfere with CPS's basic functions, or stands in the way of considerable improvements.

This post keeps track of the current backwards compatibility relationship and is updated as necessary.

Config Vars

  • IgnoreClassNames - disregarded, see below for context.
  • APRequiresTrainingCenter - disregarded, set in Mod Config Menu now.
  • RevealAllAbilities - disregarded, set in Mod Config Menu now.
  • ClassAbilitiesPerRank - fully supported, though the purpose of having it configurable is dubious.
  • ClassCustomAbilityCost - fully supported and expanded: if ClassName is left empty, the specified AbilityCost will be used for all soldier classes, expect for those who do have class-specific custom ability cost.

NPSBD Script Package

To be able to access config vars meant for the NPSBD, CPS ships the entire NPSBD script package exactly as it is in the latest NPSBD workshop version. This means that both CPS and NPSBD can be active at the same time, CPS will just quietly kill NPSBD in its sleep and take over its functions.

This has a downside: if NPBSD is ever updated for any reason, its updated script package will clash with the NPSBD script package shipped by CPS, potentially causing all kinds of issues. For this reason we have added a pop-up message (#26 and #33) that will inform the player that they're ought to disable NPSBD while using CPS.

Building against NPSBD

The main CPS class, CPS_UIArmory_PromotionHero, does not extend NPSBD_UIArmory_PromotionHero, so it is not backwards compatible with any of its functions, such as GetCustomAbilityPointCost(), which have a different function definition in CPS_UIArmory_PromotionHero.

So in a hypothetical situation, where a mod was built against NPSBD to access current promotion screen as NPSBD_UIArmory_PromotionHero, and then run some of its functions, then a mod like that would still be able to launch alongside CPS, thanks to CPS including NPSBD script package, but the mod would be unable to cast the current promotion screen to NPSBD_UIArmory_PromotionHero. It's hard to come up with a legitimate use-case for doing something like this, though.

@Iridar
Copy link
Author

Iridar commented May 27, 2021

During some of my refactoring, I have changed function definitions, but it has occurred to me that perhaps we're ought to keep the original function, just potentially change their implementation? To keep things backwards compatible, so that if a third party mod was built against original NPSBD (for whatever reason), then it can also work against CPS?

Although I'm not sure why would anybody want to build against CPS / NPSBD, to my knowledge no mods do that, and it's not like NPSBD has some sort of special logic that cannot be just copypasted.

Personally I don't think it's important to maintain backwards compatibility to that extent, just keeping it compatible with configuration for the original NPSBD should be enough for 99% of potential situations.


To add to this: CPS will ship the entire NPSBD script package unchanged, so any mod that was built against NPBSD will work with CPS as well. We're also fully backwards compatible with config intended for NPSBD, with one exception: we ignore the IgnoreClassNames config array which didn't have much (any) use in NPSBD, but in theory it could have been used by a mod to tell it to not override the promotion screen for regular soldiers. Kinda against the purpose of the mod, and we can't support it anyway, as that would make NPSBD itself disable CPS's replacement of said promotion screen.

@Iridar
Copy link
Author

Iridar commented Jun 15, 2021

On further consideration following a real use case, we should make sure that if we implement our version of functions that are present in UIArmory_PromotionHero class, such as GetAbilityPointCost(), we must make sure not to change function definitions. I'll have to inspect the code to check if we're currently doing that.

Iridar added a commit that referenced this issue Jun 16, 2021
5b866db Release v1.1.0
1dd9936 Merge pull request #28 from robojumper/remove_verification
1b88ae5 Update README with information about X2ProjectGenerator
60a8235 Rip project file verification out of build_common
6647121 Merge pull request #42 from robojumper/changelog
f6b7c1c Changelog the 2nd
702c0f4 Merge pull request #38 from robojumper/profile
f126426 Don't set the thread's culture just for a decimal dot
3f602dc Add self-profiling to report step timings
ed7a5f2 Merge pull request #31 from robojumper/check-macro-redefs
81c0216 More descriptive macro redefinition error message
0da0304 Merge pull request #39 from robojumper/future-proof
adf7f69 Use release tags in installation instructions instead of pulling main
8817f76 Don't delete XComGame\Published in clean.ps1
8da626f Error out on macro redefinitions as a stopgap measure
da647e0 Merge pull request #29 from robojumper/changelog
f825c12 Merge pull request #27 from robojumper/error-file-line-syntax
06f9d17 Add rolling changelog
0c508eb Use error syntax compatible with both ModBuddy and VS Code

git-subtree-dir: .scripts/X2ModBuildCommon
git-subtree-split: 5b866db251265ebcdc17d631de2539ab927e112e
@Iridar
Copy link
Author

Iridar commented Jun 26, 2021

I have checked around, and it doesn't look like we're changing function definitions of any functions from UIArmory_PromotionHero. We are changing some function definitions we've copied from NPBSB script package, but seeing as CPS_UIArmory_PromotionHero does not extend NPSBD_UIArmory_PromotionHero, they cannot be used interchangeably anyway by other mods, so I think it's fine to keep it this way unless there's an actual use case for doing otherwise.

@Iridar Iridar pinned this issue Jun 26, 2021
Iridar added a commit that referenced this issue Nov 17, 2021
c7e3945 Release v1.1.1
cb83114 Merge pull request #60 from pledbrook/patch-3
932381d Merge pull request #59 from X2CommunityCore/fail-on-sdk-revert-fail
d4e3e9b Merge pull request #56 from robojumper/55-spaces-in-project-path
63ff7e3 Make path rewriting aware of IncludeSrc (#57)
ccf2109 Remove /DCOPY:DA robocopy option
93da7dc Added a check (and fail) for post-cooking SDK cleanup
b78de7e Add disclaimer about spaces to README, update CHANGELOG
a72ea3d Fix mod project paths containing spaces
becea48 Merge pull request #53 from X2CommunityCore/revert-dlc-cook
305bef8 Revert DLC cooking + some improvements
3fef85d Merge pull request #51 from pledbrook/patch-1
53cc89e Fix build error when cooking maps
7bc79b5 Switched to DLC cooking functionality (#46)
5b866db Release v1.1.0
1dd9936 Merge pull request #28 from robojumper/remove_verification
1b88ae5 Update README with information about X2ProjectGenerator
60a8235 Rip project file verification out of build_common
6647121 Merge pull request #42 from robojumper/changelog
f6b7c1c Changelog the 2nd
702c0f4 Merge pull request #38 from robojumper/profile
f126426 Don't set the thread's culture just for a decimal dot
3f602dc Add self-profiling to report step timings
ed7a5f2 Merge pull request #31 from robojumper/check-macro-redefs
81c0216 More descriptive macro redefinition error message
0da0304 Merge pull request #39 from robojumper/future-proof
adf7f69 Use release tags in installation instructions instead of pulling main
8817f76 Don't delete XComGame\Published in clean.ps1
8da626f Error out on macro redefinitions as a stopgap measure
da647e0 Merge pull request #29 from robojumper/changelog
f825c12 Merge pull request #27 from robojumper/error-file-line-syntax
06f9d17 Add rolling changelog
0c508eb Use error syntax compatible with both ModBuddy and VS Code

git-subtree-dir: .scripts/X2ModBuildCommon
git-subtree-split: c7e3945067903304bf74599b763675ffecd43f79
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

No branches or pull requests

1 participant