Skip to content

Conversation

@NoPressF
Copy link
Contributor

No description provided.

@Southclaws Southclaws requested a review from Copilot August 6, 2025 12:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements NPC respawning functionality and movement towards players by adding new native functions and callbacks. The changes provide script access to respawn NPCs and move them to specific player positions.

  • Added NPC_Respawn native function and corresponding onNPCRespawn callback
  • Added NPC_MoveToPlayer native function for moving NPCs to player positions
  • Updated SDK submodule to support the new interface methods

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Server/Components/Pawn/Scripting/NPC/Natives.cpp Added native functions for NPC respawning and moving to players
Server/Components/Pawn/Scripting/NPC/Events.hpp Added OnNPCRespawn callback event handler
Server/Components/NPCs/NPC/npc.hpp Added method declarations for respawn and moveToPlayer
Server/Components/NPCs/NPC/npc.cpp Implemented respawn logic and moveToPlayer functionality
SDK Updated submodule commit to include new interface methods


void NPC::respawn()
{
if (!(player_->getState() == PlayerState_OnFoot || player_->getState() == PlayerState_Driver || player_->getState() == PlayerState_Passenger || player_->getState() == PlayerState_Spawned))
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The complex conditional with multiple OR operations is difficult to read and maintain. Consider extracting this logic into a helper method like isValidStateForRespawn() or using a set/array of valid states for cleaner comparison.

Suggested change
if (!(player_->getState() == PlayerState_OnFoot || player_->getState() == PlayerState_Driver || player_->getState() == PlayerState_Passenger || player_->getState() == PlayerState_Spawned))
if (!isValidStateForRespawn())

Copilot uses AI. Check for mistakes.
setPosition(position_, true);
setRotation(getRotation(), true);

if (isEqualFloat(getHealth(), 0.0f))
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

Using floating-point equality comparison with isEqualFloat(getHealth(), 0.0f) may not be the most appropriate check for determining if an NPC is dead. Consider using a direct check against the dead_ member variable or a specific "isDead()" method if available, as health values might not be exactly 0.0f due to floating-point precision.

Suggested change
if (isEqualFloat(getHealth(), 0.0f))
if (dead_)

Copilot uses AI. Check for mistakes.
}

bool NPC::moveToPlayer(IPlayer& player, NPCMoveType moveType, float moveSpeed)
{
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The player position is retrieved and immediately passed to the move function, but this could result in the NPC moving to a stale position if the player moves between the position retrieval and the actual movement execution. Consider implementing a more dynamic tracking mechanism or adding validation to ensure the target position is still valid.

Suggested change
{
{
targetPlayer_ = &player;

Copilot uses AI. Check for mistakes.
@AmyrAhmady
Copy link
Member

Thanks for your contributions, but unfortunately I'm going to close this because of how they're implemented and how stuff are changed now in current state of npc branch. I still appreciate it a lot. I'll implement these functions myself.

Also before creating new PRs and putting time & effort on these stuff, we need to discuss them first and see if it's okay to do them or not. I'll always guide you through it.

Thanks again.

@AmyrAhmady AmyrAhmady closed this Aug 22, 2025
@NoPressF NoPressF deleted the npc-features branch August 30, 2025 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants