SQL conversion #18

grimmier378 · 2024-10-18T23:04:58Z

Convert version 7 of ButtonMaster.lua settings to SQL DB.

Still saves to the ButtonMaster.lua file for backup purposes but will load from the DB if present

grimmier378 · 2024-10-18T23:05:58Z

forgot i was also playing with the zep editor, commented that all out and re-enabled the multiline editor.

doesn't crash when reloading the new copy.

DerpleMQ2 · 2024-11-04T02:56:11Z

lib/buttonUtils.lua


    if renderButton[key] ~= nil then
        local tColors = ButtonUtils.split(renderButton[key], ",")
-        for i, v in ipairs(tColors) do btnColor[i] = tonumber(v / 255) end


Why not just do
(tonumber(v) or 0) / 255 ?

DerpleMQ2 · 2024-11-04T02:56:46Z

lib/IconPicker.lua

    self.Open, self.Draw = ImGui.Begin('Icon Picker', self.Open, ImGuiWindowFlags.None)
    if self.Draw then
-        self.Page = ImGui.InputInt("Page", self.Page, 1)
+        self.Page = ImGui.SliderInt("Page", self.Page,1, 26)


Let's make this a tab list - i really don't like the input int or the slider its really non-obvious that you can move to different pages.

I like that idea

DerpleMQ2 · 2024-11-04T02:59:14Z

init.lua

-            flags = bit32.bor(flags, ImGuiWindowFlags.NoScrollbar)
+    if BMHotbars ~= nil then
+        for hotbarId, bmHotbar in ipairs(BMHotbars) do
+            if BMSettings:GetCharacterWindow(hotbarId) ~= nil then


we should reduce this to a single lookup

local charWindowSettings = BMSettings:GetCharacterWindow(hotbarId)

etc...

DerpleMQ2 · 2024-11-04T02:59:58Z

init.lua

    end

-    CopyLocalSet(key)
+    -- CopyLocalSet(key)


let's not add comments back into the branch.

DerpleMQ2 · 2024-11-04T03:00:15Z

init.lua

+
+    -- Save and update the settings in the database
    BMSettings:SaveSettings(true)
+    BMSettings:updateCharacterDB(BMSettings.CharConfig, BMSettings:GetCharConfig())


Can you convert this to CamelCase to match the rest of the code?

DerpleMQ2 · 2024-11-04T03:00:36Z

init.lua

    end
 end

+-- function CopyLocalSet(key)


let's not add comments back into the branch.

DerpleMQ2 · 2024-11-04T03:02:10Z

bmHotbarClass.lua

                BMSettings:GetCharacterWindow(self.id).HideTitleBar = not BMSettings:GetCharacterWindow(self.id)
                    .HideTitleBar
                BMSettings:SaveSettings(true)
+                BMSettings:updateCharacterDB(BMSettings.CharConfig, BMSettings:GetCharConfig())


why doesnt SaveSettings do updateCharacterDB?

DerpleMQ2 · 2024-11-04T03:02:47Z

bmHotbarClass.lua

            for _, value in ipairs(charList) do
                if ImGui.MenuItem(value.displayName) then
-                    CopyLocalSet(value.key)
+                    -- CopyLocalSet(value.key)


let's not add comments back into the branch.

Also is there a reason this was moved to the other coroutine? I am not against it but just curious why this was done.

DerpleMQ2 · 2024-11-04T03:02:58Z

bmEditButtonPopup.lua

    ImGui.PushFont(ImGui.ConsoleFont)
    renderButton.Cmd, textChanged = ImGui.InputTextMultiline("##_Cmd_Edit", renderButton.Cmd or "",
        ImVec2(ImGui.GetWindowWidth() * 0.98, editHeight), ImGuiInputTextFlags.AllowTabInput)
+    -- self.textEditor:Render(ImVec2(ImGui.GetWindowWidth() * 0.98, editHeight))


let's not add comments back into the branch.

DerpleMQ2 · 2024-11-04T03:04:09Z

bmEditButtonPopup.lua

    -- color pickers
-    colorChanged = btnUtils.RenderColorPicker(string.format("##ButtonColorPicker1_%s", renderButton.Label), 'Button',
-        renderButton,
+    colorChanged = btnUtils.RenderColorPicker(string.format("##ButtonColorPicker1_%s", renderButton.Label), 'Button', renderButton,


These seem like non-changes polluting the PR. Can you get rid of them please?

DerpleMQ2 · 2024-11-04T03:04:16Z

bmEditButtonPopup.lua


 function BMButtonEditor:CloseEditPopup()
    picker:SetClosed()
+    -- self.textEditor:Clear()


let's not add comments back into the branch.

DerpleMQ2 · 2024-11-04T03:05:13Z

bmEditButtonPopup.lua

                BMSettings:GetSettings().Buttons[ButtonKey].Unassigned = nil                       -- clear the unassigned flag

                BMSettings:SaveSettings(true)
+                BMSettings:updateButtonDB(btnUtils.shallowcopy(self.tmpButton), ButtonKey)


Cant all the DB updates be handle from SaveSettings? How do you know when to call updatebutton/updateset/etc? That should ideally be abstracted away as much as possible.

DerpleMQ2 · 2024-11-04T03:05:20Z

bmEditButtonPopup.lua

 BMButtonEditor.selectedTimerType   = 1
 BMButtonEditor.selectedUpdateRate  = 1
-
+-- ---@diagnostic disable-next-line:undefined-field


let's not add comments back into the branch.

DerpleMQ2 · 2024-11-04T03:05:50Z

bmButtonHandlers.lua

-    local iconId = Button.Icon
-    local iconType = Button.IconType
+    local iconId = Button.Icon or -1
+    local iconType = Button.IconType or ''


This will change existing behavior (nil iconType means something) so please make sure this doesn't break existing stuff.

DerpleMQ2 · 2024-11-04T03:06:16Z

bmButtonHandlers.lua

    local renderIconAnim = animItems
-    if iconType == nil or iconType == "Spell" then
+
+    if iconType == "Spell" then


This is likely broken as nil is a valid case. which will now be either nil or '' with your changes...

DerpleMQ2 · 2024-11-04T03:06:42Z

bmButtonHandlers.lua

        animSpellIcons:SetTextureCell(tonumber(iconId) or 0)
        renderIconAnim = animSpellIcons
-    else
+    elseif iconType == "Item" then


could do if iconType == "Item" else ... assume spell

DerpleMQ2 · 2024-11-04T03:08:02Z

bmSettings.lua

-local settings_path = settings_base .. '.lua '
+local mq                         = require('mq')
+local btnUtils                   = require('lib.buttonUtils')
+local PackageMan                 = require('mq.PackageMan')


This will break only init.lua can invoke packageman - you can use it here but you ALSO need to add it to init.lua

@grimmier378 the only reason why this doesn't fail for you is because you already have the package.

This only works here if it is a no-op. So there is no reason to use package man here. Just do a normal require and put the package man code in init.lua as derple said.

DerpleMQ2 · 2024-11-04T03:12:57Z

bmSettings.lua

+    local showLabel = icon == 0 and true or buttonData.ShowLabel ~= nil and buttonData.ShowLabel or true
+    self:saveToDB(db, [[
+    INSERT OR REPLACE INTO buttons (
+        button_number, button_label, button_render, button_text_color, button_button_color,


None of these insert or replace statements will work because they will only replace if the key collides but the only key you provide to the table is the id auto increment which is not specified here. See:
SQLite Insert or Replace
SQLite’s INSERT OR REPLACE statement is an alias for the INSERT statement with the OR clause, which specifies the action to take when a row with the same primary key already exists. When a conflict occurs, the statement deletes the existing row and inserts a new one.

DerpleMQ2 · 2024-11-04T03:13:38Z

bmSettings.lua

+
+function BMSettings:deleteButtonFromDB(id)
+    local db = self:InitializeDB()
+    self:saveToDB(db, "DELETE FROM buttons WHERE button_number = ?", id)


you are doing operations with WHERE clauses that are not keys still will result in a very slow full table scan.

DerpleMQ2 · 2024-11-04T03:13:42Z

bmSettings.lua

+
+function BMSettings:deleteSetFromDB(setName)
+    local db = self:InitializeDB()
+    self:saveToDB(db, "DELETE FROM sets WHERE set_name = ?", setName)


you are doing operations with WHERE clauses that are not keys still will result in a very slow full table scan.

DerpleMQ2 · 2024-11-04T03:13:45Z

bmSettings.lua

+
+function BMSettings:deleteButtonFromSetDB(setName, buttonNumber)
+    local db = self:InitializeDB()
+    self:saveToDB(db, "DELETE FROM sets WHERE set_name = ? AND button_number = ?", setName, buttonNumber)


you are doing operations with WHERE clauses that are not keys still will result in a very slow full table scan.

DerpleMQ2 · 2024-11-04T03:13:50Z

bmSettings.lua

+
+function BMSettings:deleteSetFromCharacterDB(charName, SetName)
+    local db = self:InitializeDB()
+    self:saveToDB(db, "DELETE FROM windows WHERE character = ? AND window_set_name = ?", charName, SetName)


you are doing operations with WHERE clauses that are not keys still will result in a very slow full table scan.

DerpleMQ2 · 2024-11-04T03:14:36Z

bmSettings.lua

+
+    -- Save Global Settings
+    if table_name == "all" or table_name == "global" then
+        self:saveToDB(db, "INSERT OR REPLACE INTO settings (server, character, settings_version, settings_last_backup) VALUES (?, ?, ?, ?)",


this likely will result in duplicates for characters as it does not specify the PK

DerpleMQ2 · 2024-11-04T03:16:10Z

bmSettings.lua

+    for _, button in ipairs(buttonsData) do
+        self.settings.Buttons[button.button_number] = {
+            Label = button.button_label,
+            highestRenderTime = button.button_render,


space these ='s out nicely please

DerpleMQ2 · 2024-11-04T03:17:25Z

bmSettings.lua

+        }
+    end
+
+    local charactersData = self:loadFromDB(db, "SELECT character, character_locked, character_hide_title FROM characters")


Can we avoid pre-loading every characters data now that we have moved to a db?

yeah that we can do. i would still pull the char names for local copy functionality but yeah no reason to have all char data loaded on one character

grimmier378 added 2 commits October 18, 2024 16:44

page picker converted to slider

cdb52a4

SQL conversion

f57e158

grimmier378 added 4 commits November 2, 2024 22:53

fix conversion

49d536e

fix missing IconType and some num/string checks

4cf5e8b

batch conversion

bd19b6a

copy local fix

c4fbab8

doesn't crash when reloading the new copy.

DerpleMQ2 reviewed Nov 4, 2024

View reviewed changes

init.lua

end

CopyLocalSet(key)

-- CopyLocalSet(key)

Copy link

Owner

DerpleMQ2 Nov 4, 2024

Choose a reason for hiding this comment

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

let's not add comments back into the branch.

DerpleMQ2 reviewed Nov 4, 2024

View reviewed changes

init.lua

end

end

-- function CopyLocalSet(key)

Copy link

Owner

DerpleMQ2 Nov 4, 2024

Choose a reason for hiding this comment

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

let's not add comments back into the branch.

DerpleMQ2 reviewed Nov 4, 2024

View reviewed changes

SQL conversion #18

Are you sure you want to change the base?

SQL conversion #18

Uh oh!

Conversation

grimmier378 commented Oct 18, 2024

Uh oh!

grimmier378 commented Oct 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brainiac Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

brainiac Nov 4, 2024 •

edited

Loading