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

refactor: Database abstraction and organization of files #1274

Merged
merged 21 commits into from
Nov 18, 2023
Merged

refactor: Database abstraction and organization of files #1274

merged 21 commits into from
Nov 18, 2023

Conversation

EmosewaMC
Copy link
Collaborator

@EmosewaMC EmosewaMC commented Nov 10, 2023 •
edited
Loading

Abstract away the database queries in a mini-orm with the following motivations in mind

  • Allow for plug and play of databases
  • Unlock even more code to be tested in the codebase that wasn't testable before due to needing an actual MySQL database connection
  • Structure database files in a client a game folder so organization is simple
  • Implement much easier ways to prepare queries via Fold Expressions and Parameter Packing in tandem with argument forwarding and inlining to allow for the same end result as before, but with a much clearer and easier to use interface.

Other changes:

  • Removed backup password checking
  • Removed a good chunk of finalize calls to sqlite queries as the destructor of result sets from cdclient queries already calls finalize in a try catch(...)
  • Remove duplicated code in Character::Character
  • Remove if statement in User::operator==
  • Simplify CreateCharacter logic for what name is used
  • PropertyManagementComponent lookups no longer use TemplateID for the property, but rather the mapId/zoneId
  • Master now shutsdown on inability to find object_id_tracker value to prevent any collisions
  • Use WriteAlignedBytes in bbb serialization for speed
  • Moved unreferenced ugc model deletion to world shutdown instead of when saving the property
  • All moved queries no longer get fields by number but rather name to prevent issues with counting
  • Fixed crash when parsing a brick count larger than the uint32_t max.

Notes:

  • LeaderboardManager is unchanged on purpose. Its change will easily be at least another 600 sloc. Will clutter the pr.
  • PropertyEntranceComponent is being redone in refactor: Property Entrance Component  #1246 and as such is not changed here
    • When both of the above have their queries added to the GameDatabase Tables, the prepared statement methods should be made completely private or removed and no part of the dlu codebase, aside from the MySQL orm connector itself should know that MySQL exists. conncpp.hpp will need to remain in its current header due to generic code, however that code should only get compiler if MySQL is switched on, which with another connector will not be done due to switches.

Tests were done incrementally for each query moved to ensure functionality and prevent hard to find breaking changes. No observed regressions were found during incremental changes. After the main pr was done, I did a lot of moving of files and structures around and, while these shouldn't have caused a regression, it is always possible that during the re-naming a new issue will pop up.

@EmosewaMC
Database: Convert to proper namespace
a0b6a51
@EmosewaMC
Database: Use base class and getter
c74e31b
@EmosewaMC
Database: Move files around
1352d9c
@EmosewaMC
Database: Add property Management query
9858ddd 
Database: Move over user queries

Tested at gm 0 that pre-approved names are pre-approved, unapproved need moderator approval
deleting characters deletes the selcted one
refreshing the character page shows the last character you logged in as
tested all my characters show up when i login
tested that you can delete all 4 characters and the correct character is selected each time
tested renaming, approving names as gm0

Database: Add ugc model getter

Hey it works, look I got around the mariadb issue.

Database: Add queries

Database: consolidate name query

Database: Add friends list query

Update name of approved names query

Documentation

Database: Add name check

Database: Add BFF Query

Database: Move BFF Setter

Database: Move new friend query

Database: Add remove friend queries

Database: Add activity log

Database: Add ugc & prop content removal

Database: Add model update

Database: Add migration queries

Database: Add character and xml queries

Database: Add user queries

Untested, but compiling code

Need to test that new character names are properly assigned in the following scenarios
gm 0 and pre-approved name
gm 0 and unapproved name
gm 9 and pre-approved name
gm 9 and unapproved name

Database: constify function arguments

Database: Add pet queries
@EmosewaMC
Database: Move property model queries
9ac28df 
Untested.  Need to test
placing a new model
moving existing one
removing ugc model
placing ugc model
moving ugc model(?)
changing privacy option variously
change description and name
approve property
can properly travel to property
@EmosewaMC
Property: Move stale reference deletion
b7d706b
@EmosewaMC
Database: Move performance update query
f5d8d8d
@EmosewaMC
Database: Add bug report query
560ac8a
@EmosewaMC
Merge branch 'main' into database-abstraction
799f006
@EmosewaMC
Database: Add cheat detection query
46220ff
@EmosewaMC
Database: Add mail send query
5816907
@EmosewaMC
Untested code
34b6b77 
need to test mailing from slash command, from all users of SendMail, getting bbb of a property and sending messages to bffs
@EmosewaMC
Merge branch 'main' into database-abstraction
e17de03
Xiphoseer
Xiphoseer previously approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@Xiphoseer Xiphoseer left a comment

Choose a reason for hiding this comment

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

Bit too much to verify that the queries are identical, but LGTM

@EmosewaMC
Copy link
Collaborator Author

Bit too much to verify that the queries are identical, but LGTM

The big thing would be the actual queries themselves

  • list the fields they get by name
  • actually get all the fields by name

@EmosewaMC EmosewaMC marked this pull request as draft November 10, 2023 13:06
@EmosewaMC
Copy link
Collaborator Author

EmosewaMC commented Nov 10, 2023 •
edited
Loading

This needs comments. Not a great PR without them. Will re-request reviews when I add comments.

@EmosewaMC EmosewaMC force-pushed the database-abstraction branch from 30aeb60 to e310538 Compare November 10, 2023 13:08
@EmosewaMC
Update CDComponentsRegistryTable.h
4b27350 
Database: Rename and add further comments

Datavbase: Add comments

Add some comments

Build: Fix PCH directories

Database: Fix time

thanks apple

Database: Fix compiler warnings

Overload destructor
Define specialty for time_t
Use string instead of string_view for temp empty string

Update CDTable.h

Property: Update queries to use mapId

Database: Reorganize

Reorganize into CDClient folder and GameDatabase folder for clearer meanings and file structure

Folders: Rename to GameDatabase

MySQL: Remove MySQL Specifier from table

Database: Move Tables to Interfaces

Database: Reorder functions in header

Database: Simplify property queries

Database: Remove unused queries

Remove extra query definitions as well

Database: Consolidate User getters

Database: Comment logs

Update MySQLDatabase.cpp

Database: Use generic code

Playkey: Fix bad optional access

Database: Move stuff around

WorldServer: Update queries

Ugc reduced by many scopes
use new queries
very fast
tested that ugc still loads

Database: Add auth queries

I tested that only the correct password can sign into an account.
Tested that disabled playkeys do not allow the user to play the game

Database: Add donation query

Database: add objectId queries

Database: Add master queries

Database: Fix mis-named function

Database: Add slash command queries

Mail: Fix itemId type

CharFilter: Use new query

ObjectID: Remove duplicate code

SlashCommand: Update query with function

Database: Add mail queries

Ugc: Fix issues with saving models

Resolve large scope blocks as well
@EmosewaMC EmosewaMC force-pushed the database-abstraction branch from 11382ab to 4b27350 Compare November 11, 2023 09:22
EmosewaMC
EmosewaMC commented Nov 11, 2023
View reviewed changes
Copy link
Collaborator Author

@EmosewaMC EmosewaMC left a comment

Choose a reason for hiding this comment

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

I have read through each file and did not observe any obvious issues.

@EmosewaMC EmosewaMC marked this pull request as ready for review November 11, 2023 11:16
aronwk-aaron
aronwk-aaron previously approved these changes Nov 11, 2023
View reviewed changes
aronwk-aaron
aronwk-aaron previously approved these changes Nov 14, 2023
View reviewed changes
@Jettford
Copy link
Collaborator

Will approve upon, Emo pushing the funnies.

@aronwk-aaron aronwk-aaron self-requested a review November 16, 2023 05:56
@aronwk-aaron aronwk-aaron merged commit 7f623d3 into main Nov 18, 2023
3 checks passed
@aronwk-aaron aronwk-aaron deleted the database-abstraction branch November 18, 2023 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jettford Jettford Jettford approved these changes

@aronwk-aaron aronwk-aaron aronwk-aaron approved these changes

@Xiphoseer Xiphoseer Awaiting requested review from Xiphoseer

@DarwinAnim8or DarwinAnim8or Awaiting requested review from DarwinAnim8or

@Wincent01 Wincent01 Awaiting requested review from Wincent01

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@EmosewaMC @Jettford @Xiphoseer @aronwk-aaron