dexyfex / CodeWalker

https://www.gta5-mods.com/tools/codewalker-gtav-interactive-3d-map
469 stars 205 forks source link

Implement bulk delete tool for grass instances #161

Closed pnwparksfan closed 2 years ago

pnwparksfan commented 2 years ago

Added bulk delete tool for grass instances.

image

dexyfex commented 2 years ago

Just a couple of questions from code review:

pnwparksfan commented 2 years ago
  • why vS2015BlueTheme1 was added to project window?

This was unintentional. Removed.

  • I wonder is there any way to use the list of ymaps that get passed into the project window, instead of iterating WorldForm.Renderer.VisibleYmaps? Maybe a timing issue making that a problem?

Not sure where you are referring to specifically. But in the "any ymap" mode it needs to access all ymaps, even those which have not been added to the project yet, so it can add new ymaps to the project if there are instances which aren't in a ymap that's part of the project yet. Is there such a list in the project window somewhere?

  • Is there any way to avoid having DeleteGrassPanel as a member of the ProjectForm? (and avoid adding other members?)

I tried doing that on my first pass, using FindPanel to get the panel when it was needed. But because it's used in several separate functions that each get called frequently (CanPaintInstances(), GetInstanceBrushRadius(), PaintGrass(), BulkEraseGrassInstancesAtMouse()), I thought it would be bad for performance (and code readability) to be searching for a panel that there'll only ever be one instance of many times each tick. I could go back to that if you prefer, but it seemed much less clean.

Other than those potential issues, looks good thanks :)

Thank you! Excited to have this functionality, and hope others make use of it too.