Skip to content

Conversation

@ramnes
Copy link
Contributor

@ramnes ramnes commented Sep 29, 2022

There are features that level admins may want to enable or disable on their level, so here we're giving them the chance to do just that with:

  • the voice amplifier
  • the global chat
  • punches

image

@ramnes ramnes requested a review from Donorhan September 29, 2022 15:39
@ramnes ramnes added the enhancement New feature or request label Sep 29, 2022
@alimtunc alimtunc mentioned this pull request Oct 7, 2022
@alimtunc alimtunc force-pushed the level-permissions branch 2 times, most recently from 0288f5d to 7030281 Compare October 10, 2022 08:25
@alimtunc
Copy link
Contributor

Added settings for all other radial actions 🙌

Capture d’écran 2022-10-11 à 17 12 32

@ramnes
Copy link
Contributor Author

ramnes commented Oct 11, 2022

Nice! @YglesEyes maybe you could use this as a starting point for what we've just mentioned in #161 ?

@YglesEyes
Copy link
Contributor

Nice! @YglesEyes maybe you could use this as a starting point for what we've just mentioned in #161 ?

Yes thanks, I just wait for Donorhan back (on monday) to discuss with him before doing anything

@ramnes
Copy link
Contributor Author

ramnes commented Oct 25, 2022

@Donorhan Anything we can do to help on that one?

@@ -1,26 +1,48 @@
import { moduleType } from '../../../client/helpers';
import { canUseLevelFeature, moduleType } from '../../../lib/misc';
Copy link
Contributor

Choose a reason for hiding this comment

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

moduleType should be imported from '../../../client/helpers'


// If it's an admin, we show the item but if it's disabled for all, the action will be ignored
const isAdmin = user.roles?.admin;
if (!isAdmin && !canUseLevelFeature(user, 'reactions')) mainMenuItems.splice(0, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

splice(0,1) removes notifications not reactions

{ id: 'shout', icon: '📢', label: 'Shout', order: 40, shortcut: 55, scope: 'me' },
moduleType.RADIAL_MENU,
]);
]);
Copy link
Contributor

Choose a reason for hiding this comment

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

The bracket is misplaced, registerModules should take 2 params

{ id: 'open-console', icon: '💬', shortcut: 56, order: 41, label: 'Text', closeMenu: true, scope: 'me' },
moduleType.RADIAL_MENU,
);
]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too

registerModules([
{ id: 'send-text', icon: '💬', shortcut: 56, order: 41, label: 'Text', closeMenu: true, scope: 'other' },
moduleType.RADIAL_MENU,
]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too

@xsyann
Copy link
Contributor

xsyann commented Oct 25, 2022

Maybe we should also add a setting for tasks and inventory?

Copy link
Contributor

@Donorhan Donorhan left a comment

Choose a reason for hiding this comment

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

Sounds good to me thanks! I left a few comments, it's not blocking but it would make it easier to add permissions later I think 😊

const { x, y } = user.profile;
updateLevel(level.name, { x, y }, level.hide);
},
'change .js-voice-amplifier-select'(event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A small idea to have less code: add a data-permission="xxx" then on the JS side do something like:

'change .permission-list select'(event) {
    const { permission } = event.target.dataset;
    updateFeaturePermissionLevel(permission, event);
  },

Copy link
Contributor

Choose a reason for hiding this comment

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

So cool, I searched for something like this but couldn't find, good to know :)

hotkeys('r', { keyup: true, scope: scopes.player }, event => {
if (event.repeat) return;

const user = Meteor.user({ fields: { _id: 1, 'profile.levelId': 1, roles: 1 } });
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can make it simpler to avoid code repetition:
Have a variable local to the file and update it against the condition line 13. Then in the rest of the functions rely on this variable

</div>
<div>
<button id="level-toolbox-position" type="button" class="js-spawn-position button">Set spawn position (current {{ spawnPosition }})</button>
<div class="feature-dropdown">
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to loop over a permission array?

There are features that level admins may want to enable or disable on their
level, so here we're giving them the chance to do just that with:

* the voice amplifier
* the global chat
* punches
@alimtunc
Copy link
Contributor

@Donorhan What about this PR ? Is it good for you or need changes ?

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants