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

Overall optimization (it should work without testing but testing would be better) #101

Merged
merged 11 commits into from
Apr 13, 2021
Merged

Overall optimization (it should work without testing but testing would be better) #101

merged 11 commits into from
Apr 13, 2021

Conversation

EdoTM
Copy link
Contributor

@EdoTM EdoTM commented Apr 11, 2021

Overall optimization:

  • Replaced some deprecated Gmod functions with new ones (e.g. SetNetworked... with SetNW..., surface.ScreenWidth() with ScrW(),table.getn(tbl) with #tbl, etc.)
  • Deleted empty if and for statements
  • Replaced user messages (and server messages) with Net functions
  • Removed unused local variables
  • Replaced old dependency SetNetworkedBeamString with SetNWString
  • Tidy code

I didn't actually edit anything else other than these things.

@EdoTM EdoTM closed this Apr 11, 2021
@EdoTM EdoTM changed the title Edotm fork/optimize1 Overall optimization (it should work without testing but testing would be better) Apr 11, 2021
@EdoTM EdoTM reopened this Apr 11, 2021
@TomyLobo
Copy link

Well anyone can replace one function call with another, so just to be sure you understand what you're doing, describe what SetNetworkedBeamString is.

@thegrb93
Copy link
Contributor

SetNetworkedBeamString are just wire's implementation of SetNWString that was removed a long time ago.

@EdoTM
Copy link
Contributor Author

EdoTM commented Apr 11, 2021

Networked Beams were an attempt to optimize the data flow between server and client when treating large data (there's also a description here, a more-than-10-years-old file) but they eventually got removed (along with the NW Beam library) by AbigailBuccaneer in 2015.
Some wire extras items (such as doors and door controllers) still use these, and they do generate errors when you try to use them (actually, in some cases you can't use them due to these errors).

A fix would be changing SetNetworkedBeamString to SetNWString, as it has already been done in #74. I personally tried doors and door controllers, and with this fix they work smoothly without a single error.

@thegrb93
Copy link
Contributor

I'll have time to review tomorrow.

@TomyLobo
Copy link

TomyLobo commented Apr 12, 2021

@thegrb93 You do realize, that if you give any kind of hint on what beams are, that kinda defeats the point of that test :)
@EdoTM passed with flying colors, though :)

@thegrb93
Copy link
Contributor

Oh, thought you were asking rather than testing

@EdoTM
Copy link
Contributor Author

EdoTM commented Apr 12, 2021

I'm now removing unused net.Write*/Read*, and I noticed that in wire_modular_panel.lua there's a strange behavior in usermessages and servermessages. On line 325 of the file that is currently on the repository, a usermessage named smsgModularPanel is initialized (the name is the same of the servermessage defined on line 204. They are correlated, but I don't know if the fact of them having an identical name is actually intentional), but there's no usermessage.Hook("smsgModularPanel", ...) or umsgModularPanel in any file, i.e. there's no clientside hook called smsgModularPanel from line 325.
Am I missing something?

Edit: I've noticed that there are several problems with Modular Panels already (#63, #97) so maybe we can skip wire_modular_panel.lua and leave its implementation to #97.

lua/autorun/server/lib-gui-panel-server.lua Outdated Show resolved Hide resolved
lua/entities/base_wdr_entity/init.lua Outdated Show resolved Hide resolved
lua/entities/gmod_wire_adv_hudindicator/cl_init.lua Outdated Show resolved Hide resolved
@thegrb93
Copy link
Contributor

Looks like a 1 to 1 conversion and looks fine. If you wouldn't mind just cleaning up the unneeded comments and revert the GetConVar change, it will be perfect.

@EdoTM
Copy link
Contributor Author

EdoTM commented Apr 13, 2021

Removed unnecessary comments and reverted GetConVar(...):GetInt() with GetConVarNumber(...)

@EdoTM EdoTM requested a review from thegrb93 April 13, 2021 09:32
@thegrb93
Copy link
Contributor

Thanks. Looks good

@thegrb93 thegrb93 merged commit f7938f0 into wiremod:master Apr 13, 2021
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

3 participants