Permission Groups#2587
Conversation
| } | ||
|
|
||
| pub fn getGroup(name: []const u8) error{GroupNotFound}!*PermissionGroup { | ||
| return groups.getPtr(name) orelse return error.GroupNotFound; |
There was a problem hiding this comment.
hashmaps don't have stable pointers
There was a problem hiding this comment.
I added the assert but thats probaly not enough right? I am sorry I am not that knowledgeable to know what would be the best course of action here.
There was a problem hiding this comment.
I finally brought myself back to this pr. I have changed the hashmap from StringHashMapUnmanaged(PermissionGroup) to StringHashMapUnmanaged(*PermissionGroup).
This should from my understanding fix the problem with the stable pointers
| defer groupDir.close(); | ||
| var iterator = groupDir.iterate(); | ||
| while (try iterator.next()) |file| { | ||
| if (file.kind == .file and std.mem.endsWith(u8, file.name, ".zon")) { |
There was a problem hiding this comment.
Would be better to make two guard clauses out of that condition.
IntegratedQuantum
left a comment
IntegratedQuantum
left a comment
There was a problem hiding this comment.
In order to keep the line changes down, could you please split this into 3 PRs:
- one only adds the core groups system and saving/loading in the world
- one adds the changes to the
User - one adds the
groupcommand
| try files.cubyzDir().writeZon(path, worldData); | ||
| } | ||
|
|
||
| pub fn loadPermissionGroups(_: *ServerWorld, dir: main.files.Dir) !void { |
There was a problem hiding this comment.
I think this function should also be in permission.zig.
There was a problem hiding this comment.
I removed it completly
|
|
||
| pub fn init(allocator: NeverFailingAllocator) *PermissionGroup { | ||
| sync.threadContext.assertCorrectContext(.server); | ||
| currentId += 1; |
There was a problem hiding this comment.
Should immediately save the metadata file.
|
Ok I did a few changes, here is a summary:
|
IntegratedQuantum
left a comment
IntegratedQuantum
left a comment
There was a problem hiding this comment.
Also there are some unaddressed comments from the last review
| try self.saveWorldConfig(); | ||
|
|
||
| try self.saveAllPlayers(); | ||
| try self.savePermissionGroups(); |
There was a problem hiding this comment.
Is force save called frequently enough? I think it may not be.
Is there any reason against immediately saving on change?
There was a problem hiding this comment.
I think its just my thinking of not having these "different" logics mixed up. I have added to world update that the groups are saved. But if you really want it I can just save where I currently have updated = true
4 participants
Follow Up PR for #2530
This adds the rest from the
permissionLayerPR which was split of. For Documentation see #2530