Skip to content

Commit

Permalink
Object leak/free fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
dsrw committed Oct 13, 2023
1 parent d47206f commit a010cf2
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 27 deletions.
2 changes: 1 addition & 1 deletion enu.nimble
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ else:
requires "nim >= 1.6.10",
"https://github.com/arnetheduck/nim-results#f3c666a",
"https://github.com/dsrw/godot-nim#892c482",
"https://github.com/dsrw/model_citizen 0.18.6",
"https://github.com/dsrw/model_citizen 0.18.8",
"https://github.com/dsrw/nanoid.nim 0.2.1",
"cligen 1.6.0",
"https://github.com/treeform/pretty",
Expand Down
9 changes: 4 additions & 5 deletions src/controllers/node_controllers.nim
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ proc remove_from_scene(unit: Unit) =
unit.units.clear
for child in units:
child.remove_from_scene()
unit.parent = nil

if unit.node of BuildNode:
BuildNode(unit.node).model = nil
elif unit.node of BotNode:
Expand All @@ -30,9 +30,8 @@ proc remove_from_scene(unit: Unit) =
debug "removing node", unit_id = unit.id
unit.node = nil

if unit of Build: Build(unit).destroy
elif unit of Bot: Bot(unit).destroy
elif unit of Sign: Sign(unit).destroy
unit.destroy
unit.parent = nil

proc add_to_scene(unit: Unit) =
debug "adding unit to scene", unit = unit.id
Expand Down Expand Up @@ -103,7 +102,7 @@ proc find_nested_changes(parent: Change[Unit]) =
find_nested_changes(change)
elif Added in change.changes:
# FIXME: this is being set for the worker thread in script_controller
change.item.parent = parent.item
change.item.fix_parents(parent.item)
change.item.add_to_scene()
elif Removed in change.changes:
reset_nodes()
Expand Down
7 changes: 3 additions & 4 deletions src/controllers/script_controllers/worker.nim
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ proc change_code(self: Worker, unit: Unit, code: Code) =
var edits = unit.shared.edits
for id in edits.value.keys:
if id != unit.id and edits[id].len == 0:
let edit = edits[id]
edits.del id
edit.destroy

unit.reset()
state.pop_flag ConsoleVisible
Expand Down Expand Up @@ -128,7 +130,7 @@ proc watch_units(self: Worker,
body(unit, change, added, removed)
if added:
# FIXME: this is being set for the main thread in node_controller
unit.parent = parent
unit.fix_parents(parent)
unit.frame_created = state.frame_count
unit.collisions.track proc(changes: seq[Change[(string, Vector3)]]) =
unit.script_ctx.timer = get_mono_time()
Expand Down Expand Up @@ -267,9 +269,6 @@ proc worker_thread(params: (ZenContext, GameState)) {.gcsafe.} =
while i < state.units.len:
if state.units[i].id == \"player-{ctx_name}":
var player = Player(state.units[i])
if player.units.len > 0:
Sign(player.units[0]).owner = nil
player.units.clear
state.units.del i
else:
i += 1
Expand Down
3 changes: 3 additions & 0 deletions src/models/bots.nim
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ method reset*(self: Bot) =
self.velocity = vec3()
self.units.clear()

method destroy*(self: Bot) =
self.destroy_impl

proc init*(_: type Bot, id = "bot_" & generate_id(), transform = Transform.init,
clone_of: Bot = nil, global = true, parent: Unit = nil): Bot =

Expand Down
3 changes: 3 additions & 0 deletions src/models/builds.nim
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,9 @@ method ensure_visible*(self: Build) =
self.start_color
self.draw(vec3(), (Computed, color))

method destroy*(self: Build) =
self.destroy_impl

proc init*(_: type Build,
id = "build_" & generate_id(), transform = Transform.init,
color = default_color, clone_of: Unit = nil, global = true,
Expand Down
7 changes: 6 additions & 1 deletion src/models/players.nim
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ proc init*(_: type Player): Player =
input_direction_value: ~Vector3,
cursor_position_value: ~((0, 0))
)
result.init_unit
result.init_unit(shared = false)
result.global_flags += Global

method on_begin_turn*(self: Player, direction: Vector3, degrees: float,
Expand Down Expand Up @@ -40,3 +40,8 @@ proc `open_code=`*(self: Player, code: string) =
unit.message = code
unit.global_flags += Visible
return

method destroy*(self: Player) =
if self.units.len > 0:
Sign(self.units[0]).owner = nil
self.units.clear
9 changes: 6 additions & 3 deletions src/models/signs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@ proc init*(_: type Sign, message: string, more = "", owner: Unit,
id: "sign_" & generate_id(),
message_value: ~message,
more_value: ~more,
glow_value: ~0.0,
width_value: ~width,
height_value: ~height,
size_value: ~size,
billboard_value: ~billboard,
frame_created: state.frame_count,
color_value: ~action_colors[black],
start_color: action_colors[black],
start_transform: transform,
owner_value: ~owner,
text_only: text_only
text_only: text_only,
parent: owner
)
self.init_unit
result = self
Expand All @@ -37,3 +37,6 @@ method main_thread_joined*(self: Sign) =
elif Hover.removed:
self.local_flags -= Highlight
state.pop_flag ReticleVisible

method destroy*(self: Sign) =
self.destroy_impl
42 changes: 29 additions & 13 deletions src/models/units.nim
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ import godotapi / spatial
from pkg / core / godotcoretypes import Basis
import core, models / [states, colors], libs / interpreters

proc fix_parents*(self: Unit, parent: Unit) =
self.parent = parent
for unit in self.units:
unit.fix_parents(self)

proc init_shared*(self: Unit) =
ensure ?self.shared_value
if ?self.parent:
Expand All @@ -17,7 +22,7 @@ proc init_shared*(self: Unit) =
let table = ~Table[Vector3, VoxelInfo]
self.shared.edits[self.id] = table

proc init_unit*[T: Unit](self: T) =
proc init_unit*[T: Unit](self: T, shared = true) =
with self:
units = ~seq[Unit]
transform_value = ~self.start_transform
Expand Down Expand Up @@ -123,25 +128,35 @@ method on_collision*(self: Model, partner: Model, normal: Vector3) {.base, gcsaf
method off_collision*(self: Model, partner: Model) {.base, gcsafe.} =
discard

proc destroy*[T: Unit](self: T) =
method destroy*(self: Unit) {.base, gcsafe.} =
fail "override me"

proc destroy_impl*(self: Bot | Build | Sign) =
ensure ?self
if self of Sign:
# :( Sign(self) will fail to compile if T is a Build or a Bot.
Sign(Unit(self)).owner = nil

for unit in self.units:
if unit of Sign:
Sign(Unit(unit)).owner = nil
unit.destroy

# :( Parent isn't set properly for instances on the main thread
if self.parent == nil and "instance" notin self.id:
when self is Sign:
self.owner = nil

if self.parent == nil:
let shared = self.shared
shared.edits.destroy
self.shared = nil
self.shared_value.destroy
Zen.thread_ctx.free(shared)
if Zen.thread_ctx.can_free(shared).freeable:
Zen.thread_ctx.free(shared)
else:
fail \"can't free shared {shared.id} for unit {self.id}"
else:
self.shared = nil
if self.id in self.shared.edits:
let edit = self.shared.edits[self.id]
self.shared.edits.del(self.id)
edit.destroy
self.shared = nil

self.parent = nil
for field in self[].fields:
when field is Zen:
# :(
Expand All @@ -151,8 +166,9 @@ proc destroy*[T: Unit](self: T) =
if state.open_unit == self:
state.open_unit = nil

if Unit(state.open_sign) == self:
state.open_sign = nil
when self is Sign:
if state.open_sign == self:
state.open_sign = nil

Zen.thread_ctx.free(self)

Expand Down

0 comments on commit a010cf2

Please sign in to comment.