[FoxGuard] Potential Flag Revamp - Thoughts?

Plugin thread here for reference.

Right now, as it stands. Foxguard uses a weird flag system. I am starting to dislike said system, and I want to replace it. However, before I go rampaging through my code and massacring the existing system, I would like to get some feedback about whether this is actually a good idea or not.

So to start, I suppose I should give a breakdown of how the flag system works, and how I want to change it.

Here’s how it works as of right now.

All flags represent one or more events.
Flags can have multiple parents (and by extension, multiple children).
Flag parent lookup is very strange, and it breaks down sort of like this:

  • The flag in question is at the very bottom of the hierarchy, at level 0.
  • Then the flag’s parents are at level 1.
  • Then the parents of the flags at level 1 are at level 2.
  • And so on…

At each level, the flags have a specific order, determined by the order of a flags parents. This is because for each flag, the order of its parents matters. A flag can have a primary parent, and then a secondary, and then a tertiary, and so on.

Once the hierarchy is constructed, parent lookup finds the first defined flag that is closest to the flag being looked up. If the flag is already defined, then it’s just that flag. If the flag is not defined, but one or more parents are, then the closest parent is returned.

This is how FoxGuard handles flag hierarchies, and it works pretty well for most things. It’s what lets the “block” flag cover all block events. Having multiple parents allows for a “block” flag, a “click” flag, and then a “blockclick” flag that inherits from both. In this case, “click” is the primary parent and “block” is the secondary parent.

This is all well and good, before I ran into a problem.

If I have a flag for everything, the list starts to get long.

Fast.

Here is what the flags list looks like at the current moment:

ROOT(true, "root", "Root"),
BUFF(false, "buff", "Buffs", ROOT),
DEBUFF(true, "debuff", "Debuffs", ROOT),

INTERACT(true, "click", "Click", DEBUFF),
INTERACT_PRIMARY(true, "attack", "Attack", INTERACT),
INTERACT_SECONDARY(true, "interact", "Interact", INTERACT),

BLOCK(true, "block", "Blocks", DEBUFF),
BLOCK_CHANGE(true, "blockchange", "Change-Blocks", BLOCK),
BLOCK_PLACE(true, "blockplace", "Place-Blocks", BLOCK_CHANGE),
BLOCK_BREAK(true, "blockbreak", "Break-Blocks", BLOCK_CHANGE),
BLOCK_MODIFY(true, "blockmodify", "Modify-Blocks", BLOCK_CHANGE),

BLOCK_INTERACT(true, "blockclick", "Click-Blocks", INTERACT, BLOCK),
BLOCK_INTERACT_PRIMARY(true, "blockattack", "Attack-Blocks", BLOCK_INTERACT, INTERACT_PRIMARY),
BLOCK_INTERACT_SECONDARY(true, "blockinteract", "Interact-Blocks", BLOCK_INTERACT, INTERACT_SECONDARY),

ENTITY_INTERACT(true, "entityclick", "Click-Entities", INTERACT),
ENTITY_INTERACT_PRIMARY(true, "entityattack", "Attack-Entities", ENTITY_INTERACT, INTERACT_PRIMARY),
ENTITY_INTERACT_SECONDARY(true, "entityinteract", "Interact-Entities", ENTITY_INTERACT, INTERACT_SECONDARY),

PLAYER_INTERACT(true, "playerclick", "Click-Players", ENTITY_INTERACT),
PLAYER_INTERACT_PRIMARY(true, "playerattack", "Attack-Players", PLAYER_INTERACT, ENTITY_INTERACT_PRIMARY),
PLAYER_INTERACT_SECONDARY(true, "playerinteract", "Interact-Players", PLAYER_INTERACT, ENTITY_INTERACT_SECONDARY),

SPAWN_MOB(true, "spawnmob", "Spawn-Mobs", DEBUFF),
SPAWN_MOB_PASSIVE(true, "spawnmobpassive", "Spawn-Passive-Mobs", SPAWN_MOB),
SPAWN_MOB_HOSTILE(true, "spawnmobhostile", "Spawn-Hostile-Mobs", SPAWN_MOB),

PLAYER_PASS(true, "playerpass", "Player-Pass-Borders", DEBUFF),
PLAYER_ENTER(true, "playerenter", "Player-Enter", PLAYER_PASS),
PLAYER_EXIT(true, "playerexit", "Player-Exit", PLAYER_PASS),

DAMAGE_ENTITY(true, "damageentity", "Damage-Entities", DEBUFF),
DAMAGE_LIVING(true, "damageliving", "Damage-Living", DAMAGE_ENTITY),
DAMAGE_MOB(true, "damagemob", "Damage-Mobs", DAMAGE_LIVING),
DAMAGE_MOB_PASSIVE(true, "damagemobpassive", "Damage-Passive-Mobs", DAMAGE_MOB),
DAMAGE_MOB_HOSTILE(true, "damagemobhostile", "Damage-Hostile-Mobs", DAMAGE_MOB),
DAMAGE_PLAYER(true, "damageplayer", "Damage-Players", DAMAGE_LIVING),

KILL_LIVING(true, "killliving", "Kill-Living", DAMAGE_LIVING),
KILL_MOB(true, "killmob", "Kill-Mobs", KILL_LIVING, DAMAGE_MOB),
KILL_MOB_PASSIVE(true, "killmobpassive", "Kill-Passive-Mobs", KILL_MOB, DAMAGE_MOB_PASSIVE),
KILL_MOB_HOSTILE(true, "killmobhostile", "Kill-Hostile-Mobs", KILL_MOB, DAMAGE_MOB_HOSTILE),
KILL_PLAYER(true, "killplayer", "Kill-Players", KILL_LIVING, DAMAGE_PLAYER),

IGNITE_ENTITY(true, "igniteentity", "Ignite-Entities", DAMAGE_ENTITY),
IGNITE_LIVING(true, "igniteliving", "Ignite-Living", IGNITE_ENTITY, DAMAGE_LIVING),
IGNITE_MOB(true, "ignitemob", "Ignite-Mobs", IGNITE_LIVING, DAMAGE_MOB),
IGNITE_MOB_PASSIVE(true, "ignitemobpassive", "Ignite-Passive-Mobs", IGNITE_MOB, DAMAGE_MOB_PASSIVE),
IGNITE_MOB_HOSTILE(true, "ignitemobhostile", "Ignite-Hostile-Mobs", IGNITE_MOB, DAMAGE_MOB_HOSTILE),
IGNITE_PLAYER(true, "igniteplayer", "Ignite-Players", IGNITE_LIVING, DAMAGE_PLAYER),

EXPLOSION(true, "explosion", "Explosions", DEBUFF),

INVINCIBLE(false, "invincible", "Invincibility", BUFF),
UNDYING(false, "undying", "Undying", INVINCIBLE);

I don’t know about you, but THAT is a lot of flags, and it’s unwieldy. Especially when you realize that it’s only covering a small amount of behavior. If FoxGuard is to have more functionality and more flags, something has to be done.

To make it worse, the flag inheritance priorities are kinda arbitrary, which means while the priorities might seem intuitive to one person, it might not seem intuitive to the next.

This is bad. Very bad. It’s big, it’s bulky, and I don’t like it.

So I want to change it.

Before I can explain this potential new system, I want to point out another flaw of the original. Take a look at the flags.
There are a lot of repeating keywords. I see “INTERACT” a dozen times. “DAMAGE” half a dozen. There’s a lot of redundancy in the flag system. I also see increasingly long flag names for more specific events. This is also bad.

I do not want to create a system where “ZOMBIE_OPEN_WOODDOOR_JUNGLE” is an actual flag. That is just ridiculous and stupid.

So this is how the new system would work. Flags become contexts, and any event can have multiple flags.

Then, you would just have a bunch of flags like so:

ROOT,
BUFF,
DEBUFF,

INTERACT,
PRIMARY,
SECONDARY,

BLOCK,
CHANGE,
PLACE,
BREAK,
MODIFY,

ENTITY,
PLAYER,

PASS,
ENTER,
EXIT,

SPAWN,
MOB,
PASSIVE,
HOSTILE,

DAMAGE,
LIVING,

KILL,
IGNITE,

EXPLOSION,

INVINCIBLE,
UNDYING,

That is a shorter list, and there are no repeats.

So when a player catches on fire, the flags sent are:
IGNITE, PLAYER, DAMAGE, LIVING, ENTITY, DEBUFF, ROOT
In no particular order. There is effectively no hierarchy anymore. Each event is just given a set of flags telling you what it is, and what it isn’t.

That way, if you do something like DAMAGE = DENY, it will cancel ignite events too, because an ignite event IS a damage event, so it also has the DAMAGE flag, in addition to the more specific IGNITE flag.

The problem with this system is…

It’s complicated.

In order to define rules with flags like these, you would need contextual rules and matching, which would look something like this:

KILL, PLAYER = DENY
DAMAGE, ENTITY = ALLOW
INTERACT = ALLOW
BLOCK = DENY

This is well and good, except for one very subtle yet very important nuance.

The order matters.

These flags would let you damage entities, but not kill players. You could click on blocks but you can’t break them.

However, If I changed the order to this:

DAMAGE, ENTITY = ALLOW
KILL, PLAYER = DENY
BLOCK = DENY
INTERACT = ALLOW

Suddenly, you can kill players again, but you can no longer click on blocks.
The entries themselves never changed, but the order did, and that is important.
While this system is EXTREMELY POWERFUL, it can also be EXTREMELY CONFUSING, especially for people who already have trouble wrapping their heads around the flag system.

However, it can also be extremely intuitive. You can apply rules to events that match desired areas of control.

If you want to prevent passive mobs from catching on fire, you just use the flags IGNITE, MOB, and PASSIVE in one entry, and it will do exactly as you want it to.

This new system could be a lot better, and it would allow me to keep the number of flags significantly lower.

There is one other issue though. This new system is going to be a tad bit slower. Because each matching rule needs to be checked starting from the first one until a match is found. While caching will make this faster, It wouldn’t be as fast as it is right now.

Finally, it would require a major rewrite of how handlers currently work in foxguard, They would be fundamentally incompatible with older handlers. However, this will also mean that handlers will get some attention, and would be a lot better than before, so it’s not all bad.

Now that I’m finally done brain-vomiting onto the page, I want to know what you guys think I should do. I’m convinced that the new system would be a lot better in terms of power, but I am not convinced that I would be a lot better for basic users to use.

So… thoughts guys? <3

3 Likes

This matches my thoughts with designing a theoretical permissions system.

Was thinking about groups just being like classes in CSS, optionally following specificity priority rules, using defined order as a secondary order.

Treating flags as contexts, is very similar, and I believe you could also use groups / permission nodes as contexts when players are involved as well.

e.g. does

Kill, player[myserver.pvptoggle=true] = ALLOW
Kill, player[myserver.pvptoggle=false] = DENY

And all the pvpToggle plugin would do is grant or revoke permissions to kill and be killed.

So a system like that would allow flags to be keys that have potential values.

I can see the use case for that, but i think that functionality can be moved elsewhere. The permissions handler for example would be able to do exactly that. For the sake of performance, I think flags should stay without values.

Yeah that was largely a second thought, but I believe that the primary idea that you posted still stands.

Any other thoughts from other people? Maybe?

From my view as a developer and server admin the proposal seems far more intuitive, especially since i never could understand the old system at all. i can also see it becoming quite slow, especially for large servers with many regions, however, i also think that the old system would eventually reach a point of slowdown anyway.

Just my unusable Australian currency

I don’t know about it being THAT slow. There are only so many combinations of flags that foxguard can spit out, so everything can be cached, which speeds it up a lot. It also has nothing to do with regions, only handlers, and thankfully most servers have only a few handlers. (Many regions + few handlers = Foxguard working as intended). Plus, not every handler is looked up, only relevant ones, so the performance hit only happens within handlers themselves. It’s hard to say without having a profiler, but i can’t exactly afford yourkit so…

1 Like

I very want to give my opinion but i have not enough knowledge To understand correctly your project ><
Just dont Forget that your plugin is use by lots of people and for the first time i use this i was very lost x)
I know its not easy but try To stay easy-use as you can.
For the rest just do it
Do it.
(Keep good works, you make very good job.)

@gabizou, @Zidane: Thoughts either of you? Anything you guys have to say would be great.

I am still new to the plugin myself, but the system you have in place now really isn’t that difficult to understand, and what your describing above is really only a little bit more complex. So those that have used this plugin before shouldn’t have too much of an issue switching over, and those just starting up still have to learn the same amount of stuff before, just with an additional ordering.
And as you described, expanding upon the current system seems it can become quite taxing. This plugin would be forever number one if it had the full ability to define right down exact specifics. It is currently powerful, but could be even more so. If this proposed change makes your work as the developer easier when expanding upon the flag system, it kinda shouldn’t be up for too much debate. I say go for it.

When I was first reading the mod description, this was the behavior I was expecting it to have. So, I’m definitely in favor of the revamp. This seems much more intuitive and powerful to me as well as having the ability to rapidly expand and update to keep up with future versions of Minecraft and added content.

1 Like

This system has already been implemented, so yeah.

1 Like