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.

Sign up for GitHub

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

Jump to bottom

Introduce CHDLCHookManager to retrieve DLCInfos with specific implemented hooks. #1247

Closed
wants to merge 97 commits into from
Closed

Introduce CHDLCHookManager to retrieve DLCInfos with specific implemented hooks. #1247

wants to merge 97 commits into from

Conversation

remcoros
Copy link
Contributor

@remcoros remcoros commented Aug 19, 2023 •
edited
Loading

fixes #212

It's still missing proper docs and comments. but this is the idea. I've split up the commits for easier reviewing.

@remcoros
Copy link
Contributor Author

remcoros commented Aug 20, 2023 •
edited
Loading

I found another issue I am undoing now:

There are a few calls to GetDLCInfos(true) (instead of false), to retrieve only the DLCS that are new in the current campaign. For example in XComOnlineEventMgr, it is used to trigger "OnLoadedSavedGame", but only for DLCs that were not in the current campaign's history.

I will undo that change so that it uses the normal GetDLCInfos(true) again.

I suppose it's not really worth optimizing these cases, since they are rarely/used only once?

@remcoros
Copy link
Contributor Author

Another question: I need some guidance on documenting/commenting on these changes.

  • Does CHDLCHookManager need a "HL-Docs: feature:" entry? (Maybe not, because it's only an internal API?)
  • Any other places I should add more comments/issue references?

@robojumper
Copy link
Member

Personally I would go for a documentation page that documents the intent and some implementation details, explicitly stating that:

  1. The Highlander does this performance optimization
  2. Any observable difference compared to calling the hooks in all DLCInfo classes in run order is a bug.

@remcoros
Copy link
Contributor Author

Added some documentation.

Should I do a rebase --interactive to make the history clean again, or will this get squashed into one commit anyway?

@Iridar
Copy link
Contributor

Iridar commented Aug 21, 2023

It's gonna get squashed. The only time we use more than one commit per PR is when an existing base game class is added to source.

@Iridar
Copy link
Contributor

Iridar commented Aug 21, 2023

Also docs fail to build atm in this PR.

@remcoros
Copy link
Contributor Author

It's gonna get squashed. The only time we use more than one commit per PR is when an existing base game class is added to source.

There is one added file. So I'll do a final rebase after this is considered 'done'.

@Iridar
Copy link
Contributor

Iridar commented Aug 21, 2023

There is one added file

But this is not base game source code file, it's entirely new, so it doesn't matter.

@remcoros
Copy link
Contributor Author

There is one added file

But this is not base game source code file, it's entirely new, so it doesn't matter.

XComPresentationLayerBase.uc

this one from base game got added.

@Iridar
Copy link
Contributor

Iridar commented Aug 21, 2023

Ah, I didn't notice. Then yes.

@remcoros remcoros marked this pull request as ready for review August 21, 2023 13:55
@remcoros
Copy link
Contributor Author

added docs, rebased, and ready for review.

@Iridar Iridar added ready-to-review A pull request is ready to be reviewed performance labels Aug 21, 2023
@Iridar Iridar added this to the 1.27.0 milestone Aug 21, 2023
ps2guides and others added 13 commits May 3, 2024 02:26
@ps2guides @Iridar
Fix Issue X2CommunityCore#1298 - make guaranteed crit abilities displ…
1402036 
…ay as 100% crit in UI
@ps2guides @Iridar
Add XComWeapon.uc to source
2918da0
@ps2guides @Iridar
Fix Issue X2CommunityCore#1290 - fix glitching weapon animations when…
3705e59 
… it is dropped on unit death
@ps2guides @Iridar
Fix Issue X2CommunityCore#1295 - separate and localize Defense stat m…
6fd8bde 
…odifiers
@Iridar
Revert "Fix Issue X2CommunityCore#1290 - fix glitching weapon animati…
7aad263 
…ons when it is dropped on unit death"

This reverts commit 3705e59.
@Iridar
Revert "Add XComWeapon.uc to source"
3e6d38a 
This reverts commit 2918da0.
@ps2guides
Bump version to 1.28.0
4ac262f
@ps2guides @Iridar
Issue X2CommunityCore#1299 - prevent rupture bonus damage from being …
ca23b71 
…added to attacks that don't deal damage
@ps2guides
Bump version to 1.28.1
cc1ce8a
@Tedster59 @Iridar
Update ShowPromotionUI to not cast as directly
932966d 
Updates the function to cast only to UIArmory_Promotion instead of its subclasses.
Also adds missing brackets to if/else statements.
@Tedster59 @Iridar
Add event to override patrol behavior if pod is spotted in stealth
fafb0ca 
Add event to override patrol behavior if pod is spotted in stealth
@ps2guides
Bump version to v1.29.0
d19bb87
@ps2guides
Merge branch 'master' of https://github.com/X2CommunityCore/X2WOTCCom…
dc58c97 
…munityHighlander
@BlackDog86 BlackDog86 assigned BlackDog86 and remcoros and unassigned BlackDog86 Aug 3, 2024
robojumper and others added 9 commits September 5, 2024 11:45
This was referenced Sep 5, 2024
Closed
Merged
@Iridar
Copy link
Contributor

Iridar commented Sep 5, 2024

Implemented in #1385

@Iridar Iridar closed this Sep 5, 2024
@Iridar Iridar removed this from the 1.29.0 milestone Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robojumper robojumper robojumper left review comments

Assignees

@remcoros remcoros

Labels
performance ready-to-review A pull request is ready to be reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve performance on DLC Hooks (X2DownloadableContentInfo)
10 participants
@remcoros @robojumper @Iridar @BlackDog86 @ps2guides @Tedster59 @copyrite @ReshiMael @xgl1991714 @Larryturbo