Skip to content

Conversation

sacr1ficez
Copy link
Contributor

@sacr1ficez sacr1ficez commented Aug 16, 2021

  • Replaced loops.
  • Replaced event sources.
  • Localized LOD_Map table (it is not used anywhere else).
  • Replaced semicolons with commas.
  • Improved readability.

I did small test, but would be nice if someone could test it accurately.

@sacr1ficez sacr1ficez changed the title Push improvements. editor: small improvements Aug 16, 2021
@stoneage-mta
Copy link
Contributor

Hey, thanks for this PR :D

I think we must keep the event attachments in resourceRoot, or the events will be triggered by other map resources...

Imagine this scenario: you have 10 resource maps (map1, map2, ..., map10). When the resource map1 starts, it will call the server-side event requestLODsClient of each map resource below the root element (this case: map2, map3, ..., map10), and they will return the content of the tables usedLODModels to all the other maps via the client-side event setLODsClient, and all they will all apply engineSetModelLODDistance, whereas in this case we just need to sync it once and within the same resource.

@sacr1ficez
Copy link
Contributor Author

@iDannz1 yeah good point, i forgot that people could use multiple map resources, i'm used to small resources count.

Copy link
Contributor

@jlillis jlillis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and appears to work, although I don't have many maps to test with. Perhaps someone with more editor/mapping experience could check this out?

@sacr1ficez sacr1ficez requested a review from jlillis September 28, 2021 06:56
@Dutchman101 Dutchman101 merged commit 7816898 into multitheftauto:master Oct 14, 2021
@Dutchman101 Dutchman101 requested review from Dutchman101 and removed request for Dutchman101 October 14, 2021 09:50
@sacr1ficez sacr1ficez deleted the editor-fix branch October 14, 2021 12:45
@patrikjuvonen patrikjuvonen added this to the 1.6 milestone Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants