Skip to content

add inventory ticker - #391

Merged
LordIdra merged 13 commits into
masterfrom
add-inventory-ticker
Oct 26, 2025
Merged

LordIdra merged 13 commits into
masterfrom
add-inventory-ticker

Conversation

@OhmV-IR

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

Copy link
Copy Markdown
Contributor

Closes #380

@Seggan

Seggan commented Oct 7, 2025

Copy link
Copy Markdown
Member

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

Seggan commented Oct 7, 2025

Copy link
Copy Markdown
Member

Sorry misread nvm

@Seggan Seggan left a comment

Copy link
Copy Markdown
Member

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

OhmV-IR commented Oct 7, 2025 •

Copy link
Copy Markdown
Contributor Author

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

@Seggan Seggan left a comment

Copy link
Copy Markdown
Member

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

OhmV-IR commented Oct 8, 2025 •

Copy link
Copy Markdown
Contributor Author

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

OhmV-IR commented Oct 8, 2025

Copy link
Copy Markdown
Contributor Author

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

…ase/PylonInventoryItem.kt

Co-authored-by: Seggan <seggan21@gmail.com>
@OhmV-IR
OhmV-IR requested a review from Seggan October 8, 2025 21:16
* @param stack The item itself
*/
fun onTick(player: Player, stack: ItemStack)
/** Speed at which onTick is called */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Formatting (space above and should be 3 lines, like the comment for PylonInventoryItem

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not entirely sure what this was asking me to do, but I think I did it

@OhmV-IR
OhmV-IR requested a review from Seggan October 13, 2025 21:41
Comment on lines +17 to +18
/** Determines the rate at which the [onTick] method will be called.
* [onTick] will be called at [tickInterval] * [PylonConfig.inventoryTickerBaseRate] */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/** Determines the rate at which the [onTick] method will be called.
* [onTick] will be called at [tickInterval] * [PylonConfig.inventoryTickerBaseRate] */
/**
* Determines the rate at which the [onTick] method will be called.
* [onTick] will be called at [tickInterval] * [PylonConfig.inventoryTickerBaseRate]
*/

interface PylonInventoryTicker {
/**
* Called when the item is detected in the player's inventory.
* will be called at [tickInterval] * [PylonConfig.inventoryTickerBaseRate

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* will be called at [tickInterval] * [PylonConfig.inventoryTickerBaseRate
* Will be called at [tickInterval] * [PylonConfig.inventoryTickerBaseRate]

@LordIdra
LordIdra merged commit d9b6cd4 into master Oct 26, 2025
3 checks passed
@LordIdra
LordIdra deleted the add-inventory-ticker branch October 26, 2025 22:33
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

4 participants