Skip to content

[Brealing changes] Adding support of CanOpen#97

Closed
VALERA771 wants to merge 2 commits intoExMod-Team:devfrom
VALERA771:can-open
Closed

[Brealing changes] Adding support of CanOpen#97
VALERA771 wants to merge 2 commits intoExMod-Team:devfrom
VALERA771:can-open

Conversation

@VALERA771
Copy link
Copy Markdown

@VALERA771 VALERA771 commented Sep 16, 2024

Description

Describe the changes
Replacing IsAllowed with CanOpen and separate their logic.

  • IsAllowed: can player interact (if false nothing will happen) (at first always true)
  • CanOpen: can player open it (if false door/locker will indicate that access is denied) (at first may be false or true)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior? (if this is a feature change)

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Replacing IsAllowed with CanOpen and separate their logic. Users need to replace IsAllowed with CanOpen if they used IsAllowed to deny access

Other information:


Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentations

Submission checklist

  • I have checked the project can be compiled
  • I have tested my changes and it worked as expected

Patches (if there are any changes related to Harmony patches)

  • I have checked no IL patching errors in the console

Other

  • Still requires more testing

@louis1706
Copy link
Copy Markdown

Heavy breaking changes

@VALERA771
Copy link
Copy Markdown
Author

I know but this adds some good functional in my opinion. So waiting for 9 (I hope that we will have a major release one day)

@Misaka-ZeroTwo
Copy link
Copy Markdown
Collaborator

This is something I did previously(Exiled-Team/EXILED#1787), but I ended up closing it due to the break change. The name canOpen can probably be changed, because player might be closing the door. Rather than opening it.

@VALERA771
Copy link
Copy Markdown
Author

VALERA771 commented Sep 27, 2024

I can't imagine better name for it. IsAllowed represents CanInteract (in compare with your pr)

@louis1706
Copy link
Copy Markdown

The problem than i have with this pr it's than it's a very huge breaking change also everyone is adapt to use IsAllowed to allow the open or closing instead of interacting it's probably sound good but i don't thinks most dev agree with this change

@VALERA771
Copy link
Copy Markdown
Author

Currently IsAllowed determines can player affect door or not (open if closed and close if opened). I want to add value to block interaction. Maybe rename CanOpen to CanInteract and make it to work how IsAllowed is working? (Just like Misaka's pr)

@louis1706
Copy link
Copy Markdown

Currently IsAllowed determines can player affect door or not (open if closed and close if opened). I want to add value to block interaction. Maybe rename CanOpen to CanInteract and make it to work how IsAllowed is working? (Just like Misaka's pr)

yeah i thinks that the best way to deal with it

since people actually use IDeniableEvent with this one just to make unable to open/close but still getting the invalid interact sound

@Misaka-ZeroTwo
Copy link
Copy Markdown
Collaborator

Currently IsAllowed determines can player affect door or not (open if closed and close if opened). I want to add value to block interaction. Maybe rename CanOpen to CanInteract and make it to work how IsAllowed is working? (Just like Misaka's pr)

My PR actually does it in a more complex way which does allow more functionality. However, given it would break a bunch of plugins, I agree with Yamato that having CanInteract might be the best approach to this problem.

@VALERA771
Copy link
Copy Markdown
Author

Closing. #160 done already

@VALERA771 VALERA771 closed this Nov 5, 2024
@VALERA771 VALERA771 deleted the can-open branch December 20, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants