Skip to content

Conversation

OhmV-IR
Copy link
Contributor

@OhmV-IR OhmV-IR commented Oct 7, 2025

Closes #380

@Seggan
Copy link
Member

Seggan commented Oct 7, 2025

You system doesn't address the main point of different tick speeds: reducing the number of inventory iterations/item comparisons. rn it just loops over everything at the same tick speed

@Seggan
Copy link
Member

Seggan commented Oct 7, 2025

Sorry misread nvm

Copy link
Member

@Seggan Seggan left a comment

Choose a reason for hiding this comment

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

This doesn't allow for arbitrary tick speeds right?

@OhmV-IR
Copy link
Contributor Author

OhmV-IR commented Oct 7, 2025

This doesn't allow for arbitrary tick speeds right?

no, we don't want arbitrary tick speeds since the number of comparisons increases along the order of nDistinctTickSpeeds * nPlayers, so if we have a bunch of different addons each with their own tick speeds, then we have to check every item again just for that addon's speed. I talked with Idra about this and we decided that these 3 speeds would fulfill 99% of use cases

Copy link
Member

@Seggan Seggan left a comment

Choose a reason for hiding this comment

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

Hit "approve" on accident 💀

@OhmV-IR OhmV-IR requested a review from Seggan October 8, 2025 15:09
@OhmV-IR
Copy link
Contributor Author

OhmV-IR commented Oct 8, 2025

oop there's a bug where health talisman health doesn't get removed since there's no onStopTick or something like that, do not merge

@OhmV-IR
Copy link
Contributor Author

OhmV-IR commented Oct 8, 2025

oop there's a bug where health talisman health doesn't get removed since there's no onStopTick or something like that, do not merge

bug has been fixed, not going to add an onStopTick method since that's just unneeded abstraction overhead in a lot of cases

@OhmV-IR OhmV-IR requested a review from Seggan October 8, 2025 21:16
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.

PylonInventoryTicker

2 participants