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

Rewrite to EGPLib objectcontrol.lua #3038

Merged
merged 10 commits into from
Apr 15, 2024
Merged

Conversation

Denneisk
Copy link
Member

@Denneisk Denneisk commented Apr 11, 2024

Tidies up things and adds comments everywhere for good measure. Part one of an eventual refactor of EGP library entirely. (Is it really a refactor with this many API changes?)

Although I renamed the file, GH will not tolerate it, so sorry, I just felt like objectcontrol.lua was an outdated name. I can't be the only one, right? You can observe 0d75e21 for most of the diff, or view the diff on your own. For your convenience, I've exported the diff for you here. If you see a problem, check the later commits in case they're fixed.

Certain method calls were changed to function calls preceded with nil. This was just an oversight thinking certain functions were not methods, and now it just serves as a memory aid that they're not actual methods.

Asking for testers

Noteworthy changes by file:

gmod_wire_expression2/core/e2lib.lua

  • Changed E2Lib to merge any preexisting E2Lib table. This was done for convenience so I don't have to explicitly place a structure to load the EGP library after E2Lib.

gmod_wire_egp/lib/init.lua

  • Made EGP a subtable of E2Lib to avoid global pollution.
    • A global EGP still exists for backwards compatibility and will be removed once everything is done.
    • Willing to revert this if it's controversial.
  • Added EGP.HookPostInit function as a simple postinit handler
  • Added descriptions to convars and moved convars into a single table initializer before files are loaded.

gmod_wire_egp/lib/egplib/objectcontrol.lua

  • Renamed to objects.lua
  • Annotated most things inside this file
  • Condensed registered objects tracking to a single table (objects). Numerical indices track IDs while string keys track object names. Greatly simplifies getter logic.
    • Removes/replaces EGP.GetObjectByID
  • Fleshed out and tidied EGPObject base and metatable.
    • Added IsValid metatable method.
    • Annotated EGPObject type.
    • Converted some functions to not invoke EGP for future use.
    • Fixed a hazard where objects would call the baseObj.EditObject when they should be calling their own self.EditObject to initialize instead.
    • Changed baseObj.Set to call SetObject if a position key is found
    • Moved most of baseObj initialization to a single table initializer
  • Changed NULL_EGPOBJECT to be an EGPObject object instead of its own specialized metatable
  • Added EGP.IsEGPObject
  • Rewrote EGP.NewObject to fit new API
  • Rewrote EGP Object initialization logic to be cleaner and agnostic to filenames
    • Rewrote EGP.InheritObject
  • Removed EGP.EditObject
  • Tidying
    • Cleaned up EGP.HasObject
    • Cleaned up EGP.SetOrder
    • Cleaned up EGP.PerformReorder
      • Un-exposed EGP.PerformReorder_Ex
    • Cleaned up EGP.CreateObject -> EGP.Create
      • Changed signature from Entity, string|integer, table to string|integer, table, Entity
        • I don't know what I was thinking here.
    • Cleaned up homescreen generator

gmod_wire_expression2/core/egpfunctions.lua

  • Added/updated locals
  • Cleaned up some e2functions
  • Replaced EGP.HasObject with localized version
  • Replaced EGP.CreateObject with updated, localized version
    • Updated usage to simply use string name directly
    • Removed unused self.player argument
  • Replaced EGP.EditObject with EGP Object method
  • Removed old comments
  • gmod_wire_expression2/core/egpobjects.lua

  • Added/updated locals
  • Use EGPObject.IsValid for validity checking
  • Replaced EGP.HasObject with localized version
  • Replaced EGP.CreateObject with updated, localized version
    • Updated usage to simply use string name directly
    • Removed unused self.player argument
  • Replaced EGP.EditObject with EGP Object method
  • Changed egpobject remove e2functions to not change metatable

All other changes were too small/insignificant and mostly involve changing/replacing usage of a few functions.

@thegrb93
Copy link
Contributor

The hasObject postInit setting is really cumbersome. Just use EGP.HasObject instead of localizing it, or define it before including those files.

@Denneisk
Copy link
Member Author

Denneisk commented Apr 11, 2024

There's a bit of overlap in all of the libraries so it's going to be more than just hasObject, and I'd prefer to keep EGP.HasObject localized because (especially right now) it's extremely expensive and common. I could change it to use an array defined in EGP or just use a hook I guess. I'll agree that scrolling to the bottom of the code to see if you defined something is pretty bad.

@Denneisk
Copy link
Member Author

Denneisk commented Apr 11, 2024

Hopefully this is a decent compromise. Same thing just more permissive.

@thegrb93
Copy link
Contributor

Conflicts

@thegrb93 thegrb93 merged commit 42a2ab1 into wiremod:master Apr 15, 2024
1 check failed
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.

2 participants