Fixed SKIN:SchemeTextEntry not working with Panel:SetSkin #1141

Merged
merged 1 commit into from Mar 22, 2016

Projects

None yet

4 participants

@Bo98
Contributor
Bo98 commented Mar 4, 2016

My last PR worked fine for getting fonts to work - but it only worked if the default skin was changed rather than Panel:SetSkin. This wasn't something that broke in my PR - it never worked for text colours either. The reason why it didn't work is that Panel:SetSkin is typically called after vgui.Create which of course is after PANEL:Init and thus after the SKIN:SchemeTextEntry call.

This pull request fixes this issue by moving the call to PANEL:ApplySchemeSettings, just like it is in other Derma controls. The reason it wasn't there in the first place was based on how the skin's call to set the text colours could override the a call that's done when setting up the panel. This was fixed by doing || checks before sending the colours into Panel:DrawTextEntryText.

This slight change of behaviour shouldn't break anything but, if possible, this should be merged before the next update in case someone gets the wrong idea of using Panel:SetFont instead of Panel:SetFontInternal in SKIN:SchemeTextEntry.

@robotboy655 robotboy655 merged commit 3b835a0 into garrynewman:master Mar 22, 2016
@Bo98 Bo98 deleted the Bo98:schemetextentry-fix branch Mar 22, 2016
@robotboy655
Collaborator

This PR is very terrible, but the old way is also terrible, and I have no idea what to do.

@Bo98
Contributor
Bo98 commented Mar 29, 2016

What's wrong with this method? It's consistent with the other controls.

@robotboy655
Collaborator

It leaves GetTextColor & Co returning nil.

@robotboy655
Collaborator

Which is why ULX is broke.

@Bo98
Contributor
Bo98 commented Mar 29, 2016

GetTextColor is supposed to be nil IMO since that should only have been used for overriding the SKIN (see DLabel).

Maybe something like DLabel's GetColor method could be implemented here but it won't address backwards compatibility.

@bigdogmat bigdogmat referenced this pull request in TeamUlysses/ulx Mar 29, 2016
Closed

local txtCol a nil value #36

@SticklyMan

I've got a workaround that should suppress the error till you guys figure this out. Sorry bout the spew of issue reports! I should have caught this a few weeks ago, but I've been pretty busy as of late.

@Kefta
Contributor
Kefta commented Mar 29, 2016

@SticklyMan They already fixed your portion with 35b990d

@SticklyMan

@Kefta Excellent, thanks for the heads up!

@Bo98
Contributor
Bo98 commented Mar 29, 2016

How does DHTMLControls have anything to do with this issue?

@Bo98 Bo98 added a commit to Bo98/garrysmod that referenced this pull request Mar 29, 2016
@Bo98 Bo98 Fixed some backwards compatibility issues with #1141. 5e2e476
@Bo98
Contributor
Bo98 commented Mar 29, 2016

For those watching this issue, see #1147.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment