Skip to content

Conversation

@kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Oct 16, 2022

Its redundant to add TM:PE data to an asset when there are no TM:PE rules. This redundancy has some downsides:

  • If there is a bug in TMPE placing an asset would cause error even if asset creator created no TM:PE rules.
  • this could potentially cause forward/backward compatibility issues
  • performance (speed/memory)

All this inconvenience can be avoided by not recording Traffic rules if there is none.

Changes:

  • Added bool ?Get/SetHasTrafficLights() which returns null/resets if traffic light value is not forced.
  • added IsDefault() to IRecordables
  • asset data is not saved (returns null) if all records have default traffic rules.
  • null guards

TMPE.zip

@kianzarrin kianzarrin added technical Tasks that need to be performed in order to improve quality and maintainability MASS EDIT The mass edit tool Asset Editor Issue related to TM:PE support in content editors labels Oct 16, 2022
@kianzarrin kianzarrin self-assigned this Oct 16, 2022
Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

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

Nice
How big is empty data record?

Base automatically changed from clear-node to master October 28, 2022 15:58
@kianzarrin kianzarrin marked this pull request as ready for review November 18, 2022 15:19
Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

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

Looks ok, question is: Is it backward compatible with existing Move It exports, e.g. those from Allbuilds.org? I mean bool to bool? change 🤔

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Nov 19, 2022

@krzychu124 good point. I saved traffic light move it record with old TMPE and loaded with new TMPE and it worked! both off and on state.
its nice to know that binary serialization is cooperating for once!

@kianzarrin kianzarrin merged commit 8b6fd00 into master Nov 19, 2022
@kianzarrin kianzarrin deleted the default-record2 branch November 19, 2022 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Asset Editor Issue related to TM:PE support in content editors MASS EDIT The mass edit tool technical Tasks that need to be performed in order to improve quality and maintainability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants