Jump to content

For the devs: ISInventoryPaneContextMenu.createMenu


Fenris_Wolf

Recommended Posts

Would somebody PLEASE refactor this function into multiple smaller functions? Apologies if already been done in IWBMS/vehicles branch, in 38.30 its 669 lines long. The massive length makes it impossible to override small parts of it via mods without worrying it will break on future updates or across all branches.

My specific issue (which makes a perfect example) comes 423 lines into the function...where the weapon upgrades context menus come into play. I'd like to override this area, but can't without doing the whole thing. This part here is actually the source of my grief (434 lines in):

 

part:getMountOn():contains(isWeapon:getDisplayName())

 

This doesn't allow individual firearms to have the display name changed without breaking upgrades. A simple tweak that would fix that, by fetching the display name from the original script item:

 

part:getMountOn():contains(isWeapon:getScriptItem():getDisplayName())

 

But thats just my issue, but the truth is any 669 line function is overly excessive, and problematic for maintainability, error checking and bug hunting.  I realize you guys are currently busy refactoring and optimizing more critical parts of the code base, I'd be more then happy to submit a refactored version if it allows you to continue the more important work uninterrupted.

 

Link to comment
Share on other sites

Well I wouldn't technically classify it as a bug, unless it was intended that the display names could be changed and not break the upgrade context menu.  Changing display names of individual firearms isn't exactly normal and unless this behavior was previously anticipated, then the code works as expected.

My whole goal was to have it so some RP servers that manually spawn ORGM's firearm could edit stats/display names for RP puproses.

But really my point wasn't about the display names/upgrade issue itself, but the length of the function and the impossibility of overwriting specific areas of it without having a mod break on different PZ branches and updates, since its highly likely this function would change overtime as new context menus are added in.

Good idea on the crosspost.

Link to comment
Share on other sites

Just now, EnigmaGrey said:

Pretty sure you can edit item names as an admin by right clicking and going into Edit. Don't know if that'll break upgrades.

In truth I haven't tested, but the code in the initial post above says it will break.  :getMountOn() returns the gun display names the upgrade can be mounted on, and it checks if the item's current display name is in there, instead of the original display name.

 

I'm willing to bet theres larger functions in the files too, I have yet to read all the base lua files.

IMO large functions can be hazardous for development teams, especially true for functions that have a high probability for changing over time.  Sometimes large functions simply cannot be avoided, in this specific case however a refactor would probably be in the best interest of both the team and modders.

 

As stated I'd even be willing to donate my own time to do the grunt work it really wouldn't take me long, documenting the changes would take longer then the actual refactor.  But since I'm a GOG user I only have access to the 38.30 branch and not the others, if the function has been changed then anything I do would already be partially outdated.

Link to comment
Share on other sites

After a quick regex check (to satisfy my self-curiosity), there are 5977 functions in 504 files

Counting only functions over 300 lines (not counting blank lines or comments), the largest functions in order are:

MainOptions:create (1124 lines)
ISWorldObjectContextMenu.createMenu (941 lines)
MainScreen:instantiate (841 lines)
ISInventoryPaneContextMenu.createMenu (669 lines)
ISBlacksmithMenu.doBuildMenu (466 lines)
ISInventoryPage:refreshBackpacks (386 lines)
ServerList:create (378 lines)
ISInventoryPane:renderdetails (332 lines)
EDebug.DemoTime (327 lines)
 

So ISInventoryPaneContextMenu.createMenu clearly isn't the largest, its the 4th.  But with this info the same arguments for refactoring could be applied to ISWorldObjectContextMenu.createMenu as well (being almost 300 lines larger), probably even more so.

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...