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

Database refactor #63

Closed
wants to merge 4 commits into from
Closed

Database refactor #63

wants to merge 4 commits into from

Conversation

TonybynMp4
Copy link
Contributor

Description

Gotta start somewhere, please contribute to this lol

Checklist

  • I have personally loaded this code into an updated Qbox project and checked all of its functionality.
  • My code fits the style guidelines.
  • My PR fits the contribution guidelines.

config.lua Outdated Show resolved Hide resolved
Comment on lines -20 to -33
CREATE TABLE IF NOT EXISTS `bans` (
`id` int(11) NOT NULL AUTO_INCREMENT,
`name` varchar(50) DEFAULT NULL,
`license` varchar(50) DEFAULT NULL,
`discord` varchar(50) DEFAULT NULL,
`ip` varchar(50) DEFAULT NULL,
`reason` text DEFAULT NULL,
`expire` int(11) DEFAULT NULL,
`bannedby` varchar(255) NOT NULL DEFAULT 'LeBanhammer',
PRIMARY KEY (`id`),
KEY `license` (`license`),
KEY `discord` (`discord`),
KEY `ip` (`ip`)
) ENGINE=InnoDB AUTO_INCREMENT=1;
Copy link
Member

Choose a reason for hiding this comment

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

what happened to bans?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i guess we really shouldn't manage that ourselves and use txadmin's bans instead cauz txadmin actually maintains that system

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah bans should not be handled in the core cuz FUCK me that was a shit system, hopefully TXAdmin intergrates a ban so we can run it through txadmin completely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah bans should not be handled in the core cuz FUCK me that was a shit system, hopefully TXAdmin intergrates a ban so we can run it through txadmin completely

shouldn't be hard to pr exports for their moderation tools 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't be hard to pr exports for their moderation tools 🤔

The hard part is always getting the PR approved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have experienced that part

@@ -12,32 +9,16 @@ CREATE TABLE IF NOT EXISTS `players` (
`inventory` longtext DEFAULT NULL,
`last_updated` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
PRIMARY KEY (`citizenid`),
KEY `id` (`id`),
FOREIGN KEY (`userid`) REFERENCES `users` (`userid`) ON DELETE CASCADE ON UPDATE CASCADE,
Copy link
Member

Choose a reason for hiding this comment

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

Interesting discussion was had last night regarding updating cached values when deletes/updates are cascaded. While the FK constraint is great, we need to have a discussion on whether we want to cascade deletes/updates on the DB and figure out an alternate cache solution, or otherwise keep the cascades within lua code so that the cache can be easily updated.

@benzon
Copy link

benzon commented Jun 12, 2023

After trying out the changes in an environment this is not going to work.

Removing citizenid and not generating it ad the start will cause a heap of problems, since so many scripts relies on it, not sure what the appropriate solution here would be, since the current PR’ed solution seems like it’s not going to fly.

@TonybynMp4
Copy link
Contributor Author

yeah there's no way this pr works lol

@TonybynMp4 TonybynMp4 closed this Aug 20, 2023
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.

None yet

5 participants