Structurize
Structurize copied to clipboard
ldtteam
Fix allowPlayerSchematics
Closes #776
Changes proposed in this pull request
- First, I noticed that locally scanned schematics still could be uploaded to the server despite the config being on, the option was only attempting to hide local schematics, not actually blocking them server side if they still managed to show up somehow.
- Second, due to an early return if, local only schematic packs weren't actually being hidden as they should have, allowing the config option to be bypassed. This early return has been removed so each pack is compared against the server to ensure they exist on either side at all times.
- Third, as per discussion with Sam we opted to turn the configuration option defaulting to false given the extra security risk of allowing people to upload arbitrary files directly to the server.
- Fourth, to combat concerns of packs being properly disabled and scanning not working, etc. We've opted to change how pack disabling works:
- When a pack now gets disabled, you still see it in the list (compared to the prior plain removal), however they're flagged as disabled, so you cannot select the pack, stating why you cannot select the pack.
- This also changes some code regarding random pack selection, since we can't select disabled packs.
- Random pack selection can now yield no results, when there ARE packs available, just disabled. So the "no packs installed" message had to be removed. Instead you are now shown the pack selection screen directly when it cannot choose a random pack.
- At the off-chance that the switch pack screen were to be fully empty, you'd see a text stating there's no packs.
- After testing however, I found that the client always creates your user pack even if you didn't do anything, so the text realistically cannot display, might need to remove the automatic local pack creation if you're not scanning anything.
Other updates:
- The "cancel" button in the pack selection screen never functioned because an overlapping label was blocking click handlers from triggering the button
- The "selected pack" caching is now removed from the build tool window/StructurePacks class, due to the fact that these classes were getting overridden, they wouldn't be the same instance at all times, causing some weird behaviour. Selected pack has been turned into the name of the pack itself, given that the actual pack meta can be fetched with an O(1) call.
- Turned the client configuration option for "scan tool scrolling" off by default as per request by Ray.
Testing
- [X] Yes I tested this before submitting it.
- [X] I also did a multiplayer test.
Review please
Will the config being defaulted to "false" affect all deco scans as well? Or is it just for actual levelable builds?
This needs to be tested with the shape tool as well. That currently works by generating the blueprint client-side and relying on the sync mechanism to let builders load it from the server.
That was actually what made me ask, cos I know we get a second client side folder with shape tool
Will the config being defaulted to "false" affect all deco scans as well? Or is it just for actual levelable builds?
This needs to be tested with the shape tool as well. That currently works by generating the blueprint client-side and relying on the sync mechanism to let builders load it from the server.
Sam asked me this too in DMs and I confirmed that all scan tool scans are local only, so shape tool works the same way. This also means that, if the config option is off, that one cannot use the scan/shape tool to create custom scans and use them.
I wasn't personally fully happy with this change, and I can see room for a separate config option to be introduced to still allow scans, but still don't allow client side packs. But for right now we deemed this change acceptable, as the whole point of this option is to not allow the player to bring any of their own schematics to the table, and only utilize what the server allows you to bring.
So the option now works as intended.
Will the config being defaulted to "false" affect all deco scans as well? Or is it just for actual levelable builds?
This needs to be tested with the shape tool as well. That currently works by generating the blueprint client-side and relying on the sync mechanism to let builders load it from the server.
Sam asked me this too in DMs and I confirmed that all scan tool scans are local only, so shape tool works the same way. This also means that, if the config option is off, that one cannot use the scan/shape tool to create custom scans and use them.
I wasn't personally fully happy with this change, and I can see room for a separate config option to be introduced to still allow scans, but still don't allow client side packs. But for right now we deemed this change acceptable, as the whole point of this option is to not allow the player to bring any of their own schematics to the table, and only utilize what the server allows you to bring.
So the option now works as intended.
To add to this, is that especially on public servers there are some hard to control, esp in big pack bypass ways to cheat things in. So by default it's off and a server-owner with smaller private groups can just turn it on.
I’m starting to mentally prepare for all questions why the scan tool and shape tool are not working on a newly set up server Maybe the crafting recipes of those items need to be disabled if the config is off, if able? As the items don’t really work anyway (as the personal style pack is a local style pack, so it may not be used)
I’m starting to mentally prepare for all questions why the scan tool and shape tool are not working on a newly set up server Maybe the crafting recipes of those items need to be disabled if the config is off, if able? As the items don’t really work anyway (as the personal style pack is a local style pack, so it may not be used)
At the very least I can confirm that local packs are not visible in the build tool. So if you were to scan something in locally, you wouldn't see your personal pack in the list.
I’m starting to mentally prepare for all questions why the scan tool and shape tool are not working on a newly set up server Maybe the crafting recipes of those items need to be disabled if the config is off, if able? As the items don’t really work anyway (as the personal style pack is a local style pack, so it may not be used)
Thanks for mentioning that. That reminds me. @Thodor12 can we add a msg to the player if they try to scan or buildtool place. Maybe we don't disable the player pack after all, but do a "msg on attempt" to the player with "enable config on server".
Will the config being defaulted to "false" affect all deco scans as well? Or is it just for actual levelable builds?
Non-deco scans are blocked regardless of this setting. If this setting is set false, it will additionally block player decorations and the shape tool. Hence why I'm saying it should absolutely not be defaulted to false.
Also, if this is set to false, does it block the player pack in a single-player world as well? Or does it only affect true servers?
Because I don't think it should block the local pack in SP regardless of this setting ... or if it does, another reason why it should not default to false.
Also, if this is set to false, does it block the player pack in a single-player world as well? Or does it only affect true servers?
Because I don't think it should block the local pack in SP regardless of this setting ... or if it does, another reason why it should not default to false.
It should not affect singleplayer worlds @Thodor12 please make sure when testing this, that this works.
Singleplayer doesn't get affected by this because of the shared filesystem, local packs are always server packs
I wasn't personally fully happy with this change, and I can see room for a separate config option to be introduced to still allow scans, but still don't allow client side packs.
What do you mean by allow scans, but not client side packs? Client side packs can't be used anyway, they have to be on the server - only deco/scans can be used from client side.
I think separate option for the shape tool at the very least should be a thing - the shape tool is different to scans in a way, and is too useful to be totally blocked
I’m starting to mentally prepare for all questions why the scan tool and shape tool are not working on a newly set up server
Yeah, me too, is why I am asking for clarification on this as it goes!
I wasn't personally fully happy with this change, and I can see room for a separate config option to be introduced to still allow scans, but still don't allow client side packs.
What do you mean by allow scans, but not client side packs? Client side packs can't be used anyway, they have to be on the server - only deco/scans can be used from client side.
I think separate option for the shape tool at the very least should be a thing - the shape tool is different to scans in a way, and is too useful to be totally blocked
I’m starting to mentally prepare for all questions why the scan tool and shape tool are not working on a newly set up server
Yeah, me too, is why I am asking for clarification on this as it goes!
Scanning is something we can do directly from in-game, so we know what's going to happen, what we have, etc. This could be uploaded with less of a hassle compared to something that was created by someone outside of the game.
The most important thing is to have some ingame way that tells players why it's not working. So that we don't get a lot of reports.
I wasn't personally fully happy with this change, and I can see room for a separate config option to be introduced to still allow scans, but still don't allow client side packs.
What do you mean by allow scans, but not client side packs? Client side packs can't be used anyway, they have to be on the server - only deco/scans can be used from client side. I think separate option for the shape tool at the very least should be a thing - the shape tool is different to scans in a way, and is too useful to be totally blocked
I’m starting to mentally prepare for all questions why the scan tool and shape tool are not working on a newly set up server
Yeah, me too, is why I am asking for clarification on this as it goes!
Scanning is something we can do directly from in-game, so we know what's going to happen, what we have, etc. This could be uploaded with less of a hassle compared to something that was created by someone outside of the game.
Might find a way to duplicate ingame items still though.
@Talyda @MotionlessTrain Discussed with Ray, we're making it so the packs are still visible in the build tool, but disabled, and the selection buttons will show a warning on hover as to why they are disabled like so:
This should hopefully reduce some questions.