Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New NPC Api #241

Merged
merged 15 commits into from
Dec 9, 2024
Merged

New NPC Api #241

merged 15 commits into from
Dec 9, 2024

Conversation

NotZer0Two
Copy link

Description

Describe the changes
Changes the entire NPC Api to use the dummies and adds the new follow system for dummies

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

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

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Yes some method in specific Spawn() Used to have the InstanceId but now removed.

Other information:
Introduces a new way and actually vsr friendly to the npcs, removing the old system without alot of changes


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

EXILED/Exiled.API/Features/Npc.cs Outdated Show resolved Hide resolved
Copy link

@Mikihero Mikihero left a comment

Choose a reason for hiding this comment

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

might also wanna fix all the shit stylecops asking about

EXILED/Exiled.API/Features/Npc.cs Outdated Show resolved Hide resolved
@NotZer0Two
Copy link
Author

i honestly don't know, how to do it because most of the stuff is impossible to do it being something on the game not possible to cache

Copy link

@VALERA771 VALERA771 left a comment

Choose a reason for hiding this comment

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

We have had internal discussions about npcs in Player.List and came to decision that list shouldn't contain them. Can you do a separate list for npc in its class and remove them from list in player's class?

EXILED/Exiled.API/Features/Npc.cs Outdated Show resolved Hide resolved
@NotZer0Two
Copy link
Author

We have had internal discussions about npcs in Player.List and came to decision that list shouldn't contain them. Can you do a separate list for npc in its class and remove them from list in player's class?

I wanted to ask that but figure it out on just doing the fixes and nothing else and yes i will do a dictionary that is not included

@NotZer0Two NotZer0Two requested a review from VALERA771 November 30, 2024 15:35
Copy link

@VALERA771 VALERA771 left a comment

Choose a reason for hiding this comment

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

We still can use same player's class dictionary but just remove npc from List in player's class

@NotZer0Two
Copy link
Author

NotZer0Two commented Nov 30, 2024

We still can use same player's class dictionary but just remove npc from List in player's class

Like what i showed and merged on the pr?

@NotZer0Two
Copy link
Author

OH I understand

@NotZer0Two NotZer0Two requested a review from VALERA771 November 30, 2024 17:56
Copy link

@VALERA771 VALERA771 left a comment

Choose a reason for hiding this comment

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

Make it IReadonlyCollection and it'll be perfect

@NotZer0Two
Copy link
Author

Make it IReadonlyCollection and it'll be perfect

Done.

@VALERA771 VALERA771 merged commit d011853 into ExMod-Team:scpsl14 Dec 9, 2024
5 checks passed
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.

6 participants