Skip to content

Proto 1.5.1, register inner flag posts#1663

Open
Pablete1234 wants to merge 1 commit intodevfrom
proto-1.5.1
Open

Proto 1.5.1, register inner flag posts#1663
Pablete1234 wants to merge 1 commit intodevfrom
proto-1.5.1

Conversation

@Pablete1234
Copy link
Copy Markdown
Member

Fixes #1538 by registering inner flag posts, but it requires a proto bump as they may have ids that collide with other features.
Additionally given the required proto bump, i've made it so a few more inline filters/variables can make it as singleton ids: filters gliding and riptiding, and the variable worldtime

Copy link
Copy Markdown
Member

@cswhite2000 cswhite2000 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me 👍
I do wonder if there is anything else we should add to this as well given we are bumping the proto

build is failing due to formatter

Comment on lines +53 to +54
// Make more singletons have built-in default ids
Version FEATURE_SINGLETON_IDS_2 = new Version(1, 5, 1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be worthwhile to consider reserving something like a namespace of some sort (just like every decent programming language does with std::..., java.*, etc.) for builtins now that we're bumping the map proto here so we can add new ones without having to proto bump again in the future.

Otherwise, all OK by me

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i have mixed feelings about this, because if we start reserving the pgm. namespace, feels like new features will have to be pushed down into "pgm." prefix, and then when a new proto drops they can stop needing to add that prefix.

i still believe if this continues to happen, the proper solution will be to have "fallback ids" in the sense that pgm can register the names, but if the map defines it to something else, then the maps' decision wins.

Yes, this also brings potential confusion if a map overrides a base id with their own that does something completely different, its kind of a similar problem to old JS having writtable undefined, hence someone could undefined = true causing unexpected behavior everywhere, but i'm willing to hope that mapdevs are not just randomly assigning "maxbuildheight" to something completely arbitrary just to mess with the actual max build height varaible, and it would only affect their own map/xml.

Signed-off-by: Pablo Herrera <pabloherrerapalacio@gmail.com>
@Pablete1234
Copy link
Copy Markdown
Member Author

I do wonder if there is anything else we should add to this as well given we are bumping the proto

yea i'm wondering that too, these were the few that i could think of including, i'm open to suggestions if anyone else knows anything else minor that should be included

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Composite posts do not register child posts by id

3 participants