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 storage values to creatures #4495

Merged
merged 2 commits into from
Jul 9, 2023
Merged

Conversation

ranisalt
Copy link
Member

Pull Request Prelude

Changes Proposed

Issues addressed:
Moves storage values up from players to creatures, meaning that monsters and NPCs also get storage values.

Only player storage is saved to the database, since creatures are not stable between server restarts. It could be extended to save NPC storage values, though.

Implementation detail: I opted for an ordered map instead of unordered because std::unordered_map uses a lot of memory when empty, as it is optimized for high usage.

@MillhioreBT
Copy link
Contributor

MillhioreBT commented Jun 28, 2023

I think the change is beautiful and necessary, however it must be taken into account that all otland scripts will break.
because now -1 is replaced by nil

otherwise all the above scripts will start leaving -1 in the database

@MillhioreBT MillhioreBT added feature New feature or functionality enhancement Increase or improvement in quality, value, or extent and removed feature New feature or functionality labels Jun 28, 2023
@omarcopires
Copy link
Contributor

I think the change is beautiful and necessary, however it must be taken into account that all otland scripts will break.

because now -1 is replaced by nil

maybe it's something very stupid, but just adding to compat wouldn't be enough?

@MillhioreBT
Copy link
Contributor

I think the change is beautiful and necessary, however it must be taken into account that all otland scripts will break.
because now -1 is replaced by nil

maybe it's something very stupid, but just adding to compat wouldn't be enough?

sure, this is probably the way to go.

Copy link
Contributor

@EPuncker EPuncker left a comment

Choose a reason for hiding this comment

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

why nil instead of -1? 🤔

@ranisalt
Copy link
Member Author

why nil instead of -1? 🤔

Trying to sneakily fix this prehistoric fuck up

@MillhioreBT
Copy link
Contributor

why nil instead of -1? 🤔

Trying to sneakily fix this prehistoric fuck up

Add compatibility functions in compat.lua and you are good to ready 😊

@ranisalt
Copy link
Member Author

ranisalt commented Jul 8, 2023

Add compatibility functions in compat.lua and you are good to ready blush

I think the way I did it covers everything

@ranisalt ranisalt requested a review from EPuncker July 8, 2023 21:41
Copy link
Contributor

@EPuncker EPuncker left a comment

Choose a reason for hiding this comment

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

neat

@EPuncker EPuncker merged commit 66d0796 into otland:master Jul 9, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Increase or improvement in quality, value, or extent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants