itzg / minecraft-server-charts

MIT License
279 stars 144 forks source link

Spawn protection setting bugged #227

Open alexyao2015 opened 3 months ago

alexyao2015 commented 3 months ago

The spawn protection config setting cannot be set to "0" as mentioned in the docs as it is currently bugged. Setting it to 0 will cause the envmap to evaluate to false and instead exclude the SPAWN_PROTECTION environment variable rather than including it as intended.

https://github.com/itzg/minecraft-server-charts/blob/ba5b3ba7168c3ac6f416e9c00aea35d37c0d9875/charts/minecraft/templates/_helpers.tpl#L28

https://github.com/itzg/minecraft-server-charts/blob/ba5b3ba7168c3ac6f416e9c00aea35d37c0d9875/charts/minecraft/templates/deployment.yaml#L174

itzg commented 3 months ago

Probably need to introduce an envIntMap similar to the envBoolMap template function.

thomascizeron commented 6 days ago

Hi, I think it would be easier to modify the if statement in the minecraft.envMap template function to check if the value exists or the value is 0, allowing a value of 0 to render the template. This way, you can use the envMap template function to render integer values as well. Something like this:
{{- if or (index . 1) (and (int (index . 1 | toString)) (eq (int (index . 1 | toString)) 0)) }} Maybe there is a better way to do this, but i think it's simpler than creating a new template function for integer values. Happy to make a PR if you think this is a good idea.

itzg commented 6 days ago

I'm worried that still wouldn't work. A zero is just as "false-y" as the absence of a value, so I'm not sure how your snippet can differentiate and might coerce all absent values into zero integers.

thomascizeron commented 5 days ago

Right, after some tests the snippet don't work because of string casting. But this one works: {{- if or (index . 1) (kindIs "float64" (index . 1)) }} Either the value is true-y or it's a number (float64) (including 0).

Tested with: Type index . 1 Render
No value false
Null value null false
Empty string "" false
Empty array [] false
Empty map {} false
Zero integer 0 true
Non-zero integer 1 true
String "string" true
Array [1, 2] true
Map {key: value} true
itzg commented 4 days ago

Ah yes, the kindIs approach looks very promising. I wonder if that would have worked for checking existence of boolean values vs absence being false-y?

thomascizeron commented 4 days ago

yeah it could work to also resorb envBoolMap with an additional clause on the or

itzg commented 4 days ago

A PR would be great if you wanted to switch it to that.

thomascizeron commented 4 days ago

https://github.com/itzg/minecraft-server-charts/pull/241