Dynamic Actor IDs by Rozelette · Pull Request #1729 · HarbourMasters/Shipwright

Rozelette · 2022-10-10T13:31:39Z

This PR adds an important functionality for eventual custom actor mods: dynamic actor IDs. This allows us to dynamically add actors while the game is running, such as while loading mod OTRs. This is a drop-in replacement for vanilla actors, who will continue to have the same ID as the base game.

This replaces the actor DLF tables, so they have been removed, and some other cleanup was done for actor overlays.

There is no need for this to get merged anytime soon, but I wanted to get this up so that people can see it.

Build Artifacts

leggettc18 · 2022-10-11T05:05:52Z

soh/src/overlays/actors/ovl_Boss_Dodongo/z_boss_dodongo.c


 const ActorInit Boss_Dodongo_InitVars = {
-    ACTOR_EN_DODONGO,
+    ACTOR_BOSS_DODONGO,


I see this enum was renamed from ACTOR_EN_DODONGO to ACTOR_BOSS_DODONGO, but Gohma's was renamed the opposite way? I'm sure this is probably authentic jank, but if you wouldn't mind elaborating I'd appreciate it.

Actually I think I'm seeing it, King Dodongo had the regular Dodongo as it's actor ID and Gohma larvae had Queen Gohma's actor id. What effect would this have in game, if any? That seems like it would've caused some pretty big problems I would think.

A look through the code tells me that these particular actors don't have their instance's IDs checked very often, so I guess this is harmless and a good idea to take care of here.

Yep, it's a benign vanilla bug. I made sure to check that their IDs weren't used and to play through the areas where these actors were present to make sure bugs weren't introduced. I fixed the init vars so I could make the assumption that they were accurate while initializing the DB and that actor->id reflected the ActorDB entry for them. Without these changes, I'd have to do more record keeping.

soh/soh/ActorDB.h

dcvz

Approving pending the final naming on Lookup vs Retrieve

Kenix3

I'm curious why you didn't just load the original actor init struct and added support for two types of init structs. Shouldn't we be creating actor entries into the DB consistently?

Rozelette · 2022-11-11T02:27:46Z

The reason I added a new init struct was that it allows us to make changes to how we initialize the table as we add new features (custom actors) that require actors to supply additional information. Already we add descriptions to them for the actor viewer. If we used 1 common struct, that would mean we'd have to update every ActorInit whenever we change the format. With the ActorDBInit, we can populate it as we load the info from the mod files. If we change the struct, that's fine because we can still support loading older mods that, say, don't provide a value for a new field, we can just default that. When loading the ActorInits for built-in actors, we can also just default it for them.

In other words, doing it this way will save us a significant amount of pain in the future.

I will add too that that's the reason why the Add with ActorInit is private. We shouldn't use that for new actors being added.

dcvz

I like this very much! There are some suggestions about naming. They revolve around using GetEntry, AddEntry and so on.. but I would say if we aim for this to become the way to interact with Actors, we should just name them GetActor, AddActor, etc.

soh/soh/ActorDB.cpp

dcvz · 2023-03-02T08:34:17Z

soh/soh/ActorDB.cpp

+    entry.desc = desc.c_str();
+}
+
+extern "C" ActorDBEntry* ActorDB_Retrieve(const int id) {


Is ActorDB the only way now to interact with Actors? If so, I would say let's name it:

Suggested change

extern "C" ActorDBEntry* ActorDB_Retrieve(const int id) {

extern "C" ActorDBEntry* GetActorEntry(const int id) {

if not then I'd say let's go for

Suggested change

extern "C" ActorDBEntry* ActorDB_Retrieve(const int id) {

extern "C" ActorDBEntry* ActorDB_GetEntry(const int id) {

It is the only place to interact with certain information about actors, mainly for spawning them. But, I don't think GetActorEntry is appropriate. This is extern "C", so it is in the global namespace, and so I would like it to be namespaced as best we can. The Namespace_Name pattern is common, see OTRGlobals.h for a whole list of examples.

dcvz · 2023-03-02T08:34:40Z

soh/soh/ActorDB.cpp

+}
+
+// Get the ActorDB::Enty for the given actor id.
+ActorDB::Entry& ActorDB::Retrieve(const int id) {


Suggested change

ActorDB::Entry& ActorDB::Retrieve(const int id) {

ActorDB::Entry& ActorDB::GetEntry(const int id) {

There were previous discussions on naming this Retrieve to match the custom messages DB. I think naming this RetrieveEntry is good, but I don't want to break consistency with Get.

soh/soh/ActorDB.cpp

dcvz · 2023-03-02T08:38:20Z

soh/soh/ActorDB.cpp

+}
+
+// Get the id for a given actor by name.
+int ActorDB::RetrieveId(const std::string& name) {


Suggested change

int ActorDB::RetrieveId(const std::string& name) {

int ActorDB::GetActorEntryId(const std::string& name) {

dcvz · 2023-03-02T08:38:50Z

soh/soh/ActorDB.cpp

+    return &ActorDB::Instance->Retrieve(id).entry;
+}
+
+extern "C" int ActorDB_RetrieveId(const char* name) {


Same suggestion as above and as commented on the C++ counterpart

soh/soh/Enhancements/debugconsole.cpp

Rozelette · 2023-05-14T10:10:38Z

Note: there were some build failures on Linux due to different definitions of functions in cmath.h, so I removed them from functions.h. We were already using cmath.h's versions anyways, but when included in a C++ context, the functions were also noexcept, which caused the errors.

Kenix3

This looks like a fantastic update to me!

leggettc18 · 2023-05-18T15:52:03Z

Hmm, I just tested this and Link always seems to immediately fall through the floor. Maybe this doesn't work well with some of the more recent LUS changes?

EDIT: Not sure what changed but now I seem to be getting random crashes all over the place.

leggettc18 · 2023-05-18T20:01:39Z

I think I figured it out, Windows did still need those other math functions. It compiled fine without them but had all kinds of weird issues at runtime. I'll PR to Roze's branch.

leggettc18 · 2023-05-18T20:50:41Z

Also, after being able to build and run correctly on Windows, I found that this also seems to fix issue #2861

Fix missing math functions on Windows.

soh/soh/ActorDB.cpp

Co-authored-by: Adam Bird <Archez@users.noreply.github.com>

leggettc18 reviewed Oct 11, 2022

View reviewed changes

leggettc18 requested changes Oct 11, 2022

View reviewed changes

soh/soh/ActorDB.h Outdated Show resolved Hide resolved

dcvz approved these changes Nov 6, 2022

View reviewed changes

Kenix3 reviewed Nov 6, 2022

View reviewed changes

Rozelette force-pushed the actor-test branch from 138e50d to a35874c Compare November 11, 2022 02:19

briaguya0 added the merge conflicts PR has conflicts that need to be resolved before it can be merged label Dec 5, 2022

briaguya0 force-pushed the develop branch 2 times, most recently from 05bd4a3 to ba13e6b Compare January 17, 2023 05:34

Rozelette force-pushed the actor-test branch from a35874c to b9ca0cf Compare March 2, 2023 05:22

dcvz reviewed Mar 2, 2023

View reviewed changes

aMannus mentioned this pull request Mar 11, 2023

Hyper Bosses (act and move twice as fast) #2555

Merged

aMannus mentioned this pull request May 10, 2023

Hyper boss - Several actor ID fixes #2871

Merged

Rozelette force-pushed the actor-test branch 2 times, most recently from 63324f1 to 01d1452 Compare May 11, 2023 04:15

Rozelette added 5 commits May 14, 2023 04:31

Add dynamic actor IDs

dec0a98

Convert Ivan to use new actor system

b9140d5

attempt to fix linux build

758196a

attempt to fix linux build

85a5b39

merge

3b1fa0e

Rozelette force-pushed the actor-test branch from 01d1452 to 3b1fa0e Compare May 14, 2023 09:35

Cleanup

7530098

Kenix3 approved these changes May 14, 2023

View reviewed changes

Fix missing math functions on Windows.

da38c21

leggettc18 mentioned this pull request May 18, 2023

Fix missing math functions on Windows. Rozelette/Shipwright#1

Merged

Removes a stubbed function to fix non-windows builds.

ff4bb0c

Merge pull request #1 from leggettc18/actor-test

ee825dc

Fix missing math functions on Windows.

Archez reviewed Jun 1, 2023

View reviewed changes

soh/soh/ActorDB.cpp Outdated Show resolved Hide resolved

Update soh/soh/ActorDB.cpp

1d654ba

Co-authored-by: Adam Bird <Archez@users.noreply.github.com>

leggettc18 approved these changes Jun 2, 2023

View reviewed changes

leggettc18 merged commit fdf9086 into HarbourMasters:develop Jun 2, 2023

briaguya0 mentioned this pull request Jun 2, 2023

Rozelette's Dynamic ActorEntry with fix for Windows runtime issues #2896

Closed

garrettjoecox added this to the Sulu (7.1.x) milestone Jun 13, 2023

	extern "C" ActorDBEntry* ActorDB_Retrieve(const int id) {
	extern "C" ActorDBEntry* GetActorEntry(const int id) {

	extern "C" ActorDBEntry* ActorDB_Retrieve(const int id) {
	extern "C" ActorDBEntry* ActorDB_GetEntry(const int id) {

	ActorDB::Entry& ActorDB::Retrieve(const int id) {
	ActorDB::Entry& ActorDB::GetEntry(const int id) {

	int ActorDB::RetrieveId(const std::string& name) {
	int ActorDB::GetActorEntryId(const std::string& name) {

Conversation

Rozelette commented Oct 10, 2022 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Artifacts

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dcvz left a comment

Choose a reason for hiding this comment

Uh oh!

Kenix3 left a comment

Choose a reason for hiding this comment

Uh oh!

Rozelette commented Nov 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcvz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rozelette May 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Rozelette commented May 14, 2023

Uh oh!

Kenix3 left a comment

Choose a reason for hiding this comment

Uh oh!

leggettc18 commented May 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leggettc18 commented May 18, 2023

Uh oh!

leggettc18 commented May 18, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Rozelette commented Oct 10, 2022 •

edited by github-actions bot

Loading

Rozelette commented Nov 11, 2022 •

edited

Loading

Rozelette May 10, 2023 •

edited

Loading

leggettc18 commented May 18, 2023 •

edited

Loading