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

chore: Use string to access SQLite columns #1535

Merged
merged 5 commits into from
Apr 14, 2024
Merged

Conversation

EmosewaMC
Copy link
Collaborator

@EmosewaMC EmosewaMC commented Apr 7, 2024

Untested right now. Will try to test later. May do some other small improvements near this code, like removing finalizes and some char* -> string -> copy to data stuff, may not. idk atm.

fixes #1531

Copy link
Member

@Wincent01 Wincent01 left a comment

Choose a reason for hiding this comment

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

It probably wouldn't be a measurable improvement, but we could utilize static variables to store a cashed index of the fields.

@EmosewaMC
Copy link
Collaborator Author

I was planning on doing more improvements in a separate PR. this one is mostly just a number->text replacement and ideally any queries to the cdclient are replaced with an ORM table for caching

@EmosewaMC
Copy link
Collaborator Author

tested the following
combat in nimbus station with a stromling and nuckal is working and effects by the stromling play
lookup works
having a pirate hat on and transferring to another zone works (nimbus station ->gnarled forest)
set bonuses work (used 6 piece engineer with valiants)
buffs work(equipped ninjago chest and pants and reduced damage by 2)
projectiles still land and do damage
character create still chooses the correct shirts the player chose
switch multiple works (tested with valiant summoner weapon chargeup)
claiming and placing a model works
paradox teleporter in nexus tower has the correct animation time
adding the nuckal mission and smashing them gives the quest item and does not when you do not have the mission
can use mounts still
can dismantle models to bricks in the inventory

@EmosewaMC EmosewaMC marked this pull request as ready for review April 10, 2024 11:53
@EmosewaMC
Copy link
Collaborator Author

lookup still works

aronwk-aaron
aronwk-aaron previously approved these changes Apr 10, 2024
jadebenn
jadebenn previously approved these changes Apr 13, 2024
Copy link
Collaborator

@jadebenn jadebenn left a comment

Choose a reason for hiding this comment

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

nitpicks aside, looks good. you can address them if you want, or don't if you don't

dGame/dInventory/ItemSet.cpp Outdated Show resolved Hide resolved
dGame/dInventory/ItemSet.cpp Show resolved Hide resolved
@jadebenn jadebenn dismissed stale reviews from aronwk-aaron and themself via 1492e8b April 14, 2024 01:02
jadebenn
jadebenn previously approved these changes Apr 14, 2024
@jadebenn
Copy link
Collaborator

well, frick

jadebenn
jadebenn previously approved these changes Apr 14, 2024
include <array>

Revert "constexpr array"

This reverts commit 1492e8b.

Revert "include <array>"

This reverts commit 2b7a67e.

include <array>

constexpr array
@aronwk-aaron aronwk-aaron merged commit 5049f21 into main Apr 14, 2024
4 checks passed
@aronwk-aaron aronwk-aaron deleted the sqlite-string_fields branch April 14, 2024 04:41
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.

ENH: Replace numeric indexing of SQLite columns with column names
4 participants