garrysmod icon indicating copy to clipboard operation
garrysmod copied to clipboard

Implement some improvements for material select

Open dvdvideo1234 opened this issue 3 years ago • 8 comments

Instead of doing this manually:

function wipeMaterials(pMat)
  for k, v in pairs(pMat.Controls) do
    v:Remove(); pMat.Controls[k] = nil
  end; pMat.List:CleanList()
  pMat.SelectedMaterial = nil
end

dvdvideo1234 avatar Jan 22 '22 07:01 dvdvideo1234

Why do you change thing that are not the subject of PR? Your PR adds Clear function, why do you change other functions?

Also pro tip: first push all commits then post PR.

GitSparTV avatar Jan 25 '22 08:01 GitSparTV

@GitSparTV Thought it would be more ordered and convenient when I factorize it, but oh, well. Turns out I need Find selected method as well and a method that will actually update the PaintOverlay when the convar is changed or DImageButton is clicked, so I added it. This PR will not change the panel behavior in any way. It will just offer methods that were unavailable until now. If you insist that I only add the clear method, will just force push. And delete the other commits. That's why I renamed the PR name. Also I am still having problems with MatSelect:SetPos(x,y) and MatSelect:SetSize(sx,sy) when I clearly call it with intended parameters but my MatSelect panel is cut in half an I cannot figure out why it does that... I know I will wait some long years for this to be merged anyway so we have a lot of time to spare thinking about how this PR may be improved. 👍

You can see more here....

The MatSelect position and size are automatically recalculated based on the frame size, but you can try this is not updated when I change pnFrame:SetSize(iSx , iSy) to use different values. Maybe it is a method that I am not calling ?

dvdvideo1234 avatar Jan 25 '22 10:01 dvdvideo1234

I don't see why UpdatePaintOver is necessary. Your link is dead.

robotboy655 avatar Feb 14 '22 19:02 robotboy655

Hello, @robotboy655

I fixed the link. The function UpdatePaintOver can be used for the panel to activate a certain material when different DImageButton is clicked. It simply restores the old paint over method for the last clicked panel and updates the new panel. That way the user will avoid coding custom version of this making the code cleaner:

--[[
 * Changes the selected materil paint over function
 * When other one is clicked reverts the last change
]]
function LaserLib.SetMaterialPaintOver(pnMat, pnImg)
  if(SERVER) then return end
  -- Remove the current overlay
  if(pnMat.SelectedMaterial) then
    pnMat.SelectedMaterial.PaintOver = pnMat.OldSelectedPaintOver
  end
  -- Add the overlay to this button
  pnMat.OldSelectedPaintOver = pnImg.PaintOver
  pnImg.PaintOver = DATA.HOVP
  pnMat.SelectedMaterial = pnImg
end

dvdvideo1234 avatar Feb 15 '22 10:02 dvdvideo1234

But you already have a method to select a given material? (FindAndSelectMaterial) What's wrong with using that?

Also clicking on the internal DImageButtons should already handle the paint over function.

robotboy655 avatar Feb 15 '22 14:02 robotboy655

It runs a loop to select the material panel. I already know what panel I need to update, so running a loop to find it is pointless in my case. It will not harm the way the panel works in every way. It just consolidated the loop in one method FindByValue and updating the actual panel in the other UpdatePaintOver. Calling MatSelect:UpdatePaintOver(DImageButton) can replace the code I've sent in my last comment. It adds an extra layer of factorization and exposes the direct material update part UpdatePaintOver, so I can avoid coding a local version of the following and utilize the build in panel selector switch:

local border = 0
local border_w = 8
local matHover = Material( "gui/ps_hover.png", "nocull" )
local boxHover = GWEN.CreateTextureBorder( border, border, 64 - border * 2, 64 - border * 2, border_w, border_w, border_w, border_w, matHover )

-- This function is used as the paint function for selected buttons.
local function HighlightedButtonPaint( self, w, h )
	boxHover( 0, 0, w, h, color_white )
end

I override the DImageButton DoClick method so every time I click on material I directly switch to it and update the convar accordingly without the need to loop trough all materials. I already know what to update and where, so I am just calling it to avoid the loop :+1:

  -- Read the controls tabe and craete index
  local tCont, iC = pnMat.Controls, 0
  -- Update material panel with ordered values
 local tRow, pnImg = sort[iD]
  if(tRow.Draw) then -- Drawing is enabled
    local sCon = LaserLib.GetSequenceInfo(tRow, sort.Info)
    local sInf, sKey = sCon.." "..tRow.Key, tRow.Key
    pnMat:AddMaterial(sInf, sKey); iC = iC + 1; pnImg = tCont[iC]
    function pnImg:DoClick()
      LaserLib.SetMaterialPaintOver(pnMat, self)
      LaserLib.ConCommand(nil, sort.Sors, sKey)
      pnFrame:SetTitle(sort.Name.." > "..sInf)
    end
    function pnImg:DoRightClick()
      -- Sort the list of materials or do something other with it

The library function does the exact same thing the method SetMaterialPaintOver does :+1:

dvdvideo1234 avatar Feb 15 '22 18:02 dvdvideo1234

Why are you so afraid of an extra loop on selection like it's gonna explode your PC?

Why not just update the base panel itself so it doesn't perform that loop on selection instead of trying to add pointless methods to the base game so that only your addon then can avoid the loop?

robotboy655 avatar Feb 16 '22 13:02 robotboy655

That is because I grow that hash table with a rate that every player will eventually contribute to adding stuff it will just keep growing to be able to preform fast hash lookup-s and retrieve proper medium data. Currently it has about 30 elements, but with every beam that hits this will grow larger. The problem is that I like how the selected material overrides the paint over method and I do not want to define my own, so I aim to use the one already defined ones. I could add my own methods to the panel, but I have to rewrite the active image button as well because I will not have access to boxHover or matHover. I also need to be able to clear the panel so I can rebuild it afterwards. This actually makes the context menus manipulation quite neat and allows sorting. Besides this panel does not have a Clear() method to begin with.

dvdvideo1234 avatar Feb 16 '22 15:02 dvdvideo1234