Skip to content

Commit a644d94

Browse files
Goober5000claude
andcommitted
Remove redundant m_pos from CJumpNode; add GetRadius() accessor
CJumpNode stored a private m_pos field that duplicated the position already held by its Objects[] entry. The field was set once during construction and never updated afterward, so any subsequent movement (e.g. FRED dragging) would leave m_pos stale while Objects[m_objnum].pos held the authoritative value. Every external caller already used GetSCPObject()->pos instead; GetPosition() had zero call sites. - Remove the m_pos field and its initialization in both constructors, the move constructor, and the move-assignment operator. - Pass the constructor's position argument directly to obj_create() instead of routing through m_pos. - Repoint GetPosition() at Objects[m_objnum].pos so it always returns the live, up-to-date position. - Add GetRadius() to expose the cached m_radius, and use it in jumpnode_get_which_in() instead of the redundant model_get_radius() call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3f85d61 commit a644d94

2 files changed

Lines changed: 24 additions & 23 deletions

File tree

code/jumpnode/jumpnode.cpp

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,46 +19,40 @@ SCP_list<CJumpNode> Jump_nodes;
1919
* Constructor for CJumpNode class, default
2020
*/
2121
CJumpNode::CJumpNode()
22-
{
22+
{
2323
gr_init_alphacolor(&m_display_color, 0, 255, 0, 255);
2424

2525
m_name[0] = '\0';
2626
m_display[0] = '\0';
27-
28-
m_pos.xyz.x = 0.0f;
29-
m_pos.xyz.y = 0.0f;
30-
m_pos.xyz.z = 0.0f;
3127
}
3228

3329
/**
3430
* Constructor for CJumpNode class, with world position argument
3531
*/
3632
CJumpNode::CJumpNode(const vec3d* position)
37-
{
38-
Assert(position != NULL);
39-
33+
{
34+
Assertion(position != nullptr, "Position should not be null!");
35+
if (position == nullptr)
36+
position = &vmd_zero_vector;
37+
4038
gr_init_alphacolor(&m_display_color, 0, 255, 0, 255);
41-
39+
4240
// Set m_name and m_display
4341
sprintf(m_name, XSTR( "Jump Node %d", 632), Jump_nodes.size());
4442
m_display[0] = '\0';
45-
43+
4644
// Set m_modelnum and m_radius
4745
m_modelnum = model_load(NOX(JN_DEFAULT_MODEL), nullptr, ErrorType::WARNING);
4846
if (m_modelnum == -1) {
4947
Warning(LOCATION, "Could not load default model for %s", m_name);
5048
} else {
5149
m_radius = model_get_radius(m_modelnum);
5250
}
53-
54-
m_pos.xyz.x = position->xyz.x;
55-
m_pos.xyz.y = position->xyz.y;
56-
m_pos.xyz.z = position->xyz.z;
57-
51+
5852
// Create the object
59-
flagset<Object::Object_Flags> default_flags;
60-
default_flags.set(Object::Object_Flags::Renders);
61-
m_objnum = obj_create(OBJ_JUMP_NODE, -1, -1, NULL, &m_pos, m_radius, default_flags);
53+
flagset<Object::Object_Flags> default_flags;
54+
default_flags.set(Object::Object_Flags::Renders);
55+
m_objnum = obj_create(OBJ_JUMP_NODE, -1, -1, nullptr, position, m_radius, default_flags);
6256

6357
if (m_modelnum >= 0) {
6458
// set up animation in case of instrinsic_rotate
@@ -81,7 +75,6 @@ CJumpNode::CJumpNode(CJumpNode&& other) noexcept
8175
other.m_fred_layer = "Default";
8276

8377
m_display_color = other.m_display_color;
84-
m_pos = other.m_pos;
8578

8679
strcpy_s(m_name, other.m_name);
8780
strcpy_s(m_display, other.m_display);
@@ -106,7 +99,6 @@ CJumpNode& CJumpNode::operator=(CJumpNode&& other) noexcept
10699
other.m_fred_layer = "Default";
107100

108101
m_display_color = other.m_display_color;
109-
m_pos = other.m_pos;
110102

111103
strcpy_s(m_name, other.m_name);
112104
strcpy_s(m_display, other.m_display);
@@ -160,6 +152,14 @@ int CJumpNode::GetModelNumber() const
160152
return m_modelnum;
161153
}
162154

155+
/**
156+
* @return Radius of jump node model
157+
*/
158+
float CJumpNode::GetRadius() const
159+
{
160+
return m_radius;
161+
}
162+
163163
/**
164164
* @return Index into Objects[]
165165
*/
@@ -190,7 +190,8 @@ const color &CJumpNode::GetColor() const
190190
*/
191191
const vec3d *CJumpNode::GetPosition() const
192192
{
193-
return &m_pos;
193+
Assert(m_objnum != -1);
194+
return &Objects[m_objnum].pos;
194195
}
195196

196197
/*
@@ -536,7 +537,7 @@ CJumpNode *jumpnode_get_which_in(const object *objp)
536537
if(jnp->GetModelNumber() < 0)
537538
continue;
538539

539-
radius = model_get_radius( jnp->GetModelNumber() );
540+
radius = jnp->GetRadius();
540541
dist = vm_vec_dist( &objp->pos, &jnp->GetSCPObject()->pos );
541542
if ( dist <= radius ) {
542543
return &(*jnp);

code/jumpnode/jumpnode.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ class CJumpNode
4545

4646
int m_flags {0};
4747
color m_display_color; // Color node will be shown in (Default:0/255/0/255)
48-
vec3d m_pos;
4948
SCP_string m_fred_layer = "Default"; // FRED view layer assignment
5049

5150
CJumpNode(const CJumpNode&);
@@ -65,6 +64,7 @@ class CJumpNode
6564
const char *GetName() const;
6665
const char *GetDisplayName() const;
6766
int GetModelNumber() const;
67+
float GetRadius() const;
6868
int GetSCPObjectNumber() const;
6969
int GetPolymodelInstanceNum() const;
7070
const object *GetSCPObject() const;

0 commit comments

Comments
 (0)