I'm writing Yet Another HAM Editor For Descent II. aaa
#11
For kp's analysis, I've created two Descent 2 mission files demonstrating oddities

The contents are as follows
Suzanne Test: Tests the "Suzanne" model earlier in the thread. This one will either crash immediately upon selecting the mission in the menu, or it won't crash at all. This suggests a memory problem, I suspect in some cases the memory layout is such that the negative SOBJCALLs don't cause a segfault, or end up in memory that doesn't cause any invalid IDTA commands to be executed (maybe it lands on a 0x0000)

Player Model Test: The player model shown in the previous post. It mostly works, but the extra lasers from the Quad Lasers don't fire under certain circumstances. Though I forgot to add one to the test level, I modified the Guided Missile with 0 speed to serve as a camera, though due to a oversight it only applies on Hotshot difficulty. Oops.

As a side note, there's a small other (non-bug related, this is testing intentional features) test to show the model, the Player Model Test mission includes files to enable the Descent 1 mine escape sequence in D2X-Rebirth.
Reply
#12
Confirmed crash for Suzanne. I got invalid polygon model, then when retrying, got an out of range memory access. I've known for several years that invalid models can cause it to access outside the buffer, but fixing it has been low priority because it's hard to exploit for anything other than a simple crash and it's hard to fix, especially if I want to avoid rechecking the model every time it is drawn. I probably can't do much with this until the weekend.

If you choose to do your own builds, and you are working with customized data, you may want to compile with -fsanitize=address so that the game is instrumented with range-checking code. This makes the program bigger and slower, but you get clean error checking on bounds violations.

I get an unrelated crash on playermodeltest: you cannot exceed the DOS 8.3 filename limits without breaking the "highest level played" code. I can prevent the crash easily, but fixing this properly requires either blocking missions with excessive names or replacing the data storage with a format that can handle longer names.
Reply
#13
if it's going to be a perf hit, it's probably not worth checking SOBJCALL jumps at runtime, provided that there aren't any potential security issues associated with it. At the moment my exporter should error out on a jump getting too long (the exporter is now properly using signed offsets, also), and hopefully I can get the chaining concept working.

re: 8.3 filenames... wooow, heh, I haven't thought about those kinds of problems in a while, heeh... Too used to things like GZDoom where a lot of those limitations have been excised. Oops. I'm curious why I didn't get any crashes on my end. hmm

Also, I have one other issue I'm having trouble tracking down, the ship model has GLOW calls associated with them, and as expected it properly makes the red bits glow, but it also seems to be affecting other polygons. My assumption was that it only affected one face, but I might have misinterpreted the interpreter code regarding GLOW calls. hmm.

EDIT: So I got the bitmaps.tbl generator made, and can export all the game's data and models for it, and I have a working editor build, but it would appear that Parallax has changed stuff heavily in Descent 2 compared to the Descent 1 editor, which I've played with quite a bit and have a working bitmaps.tbl file for. (I should make sure that works in rebirth...) At the moment it seems the thing can't implicitly load images from the piggy file, and I'm not quite sure how to tell it to. In theory I could dump all the images of the game to ILBMs to try to make it happy, but this seems weird. I'm probably missing something.
EDIT2: hmm, I'm getting a lot of segfaults trying to parse something the old dos descent 1 editor build parses just fine. Presumably Rebirth can still handle descent 1 1.0's BITMAPS.BIN file, so I'll look to see what's different and try some debugging to figure out what's going on. EDIT3: ugh this is gonna be one of those wall of edits, GDB is pointing the segfault at the atof function. Interesting. Some tiny edits to the file allow it to be parsed (I had to remove all \ newline escapes and monkey around with the AI defintions before it would go. I still have the original file for comparison and further debugging, of course)

edit4: I suspect with Descent 2 things were changed so that you do need to have all the ILBMs dumped in order to use the editor and parse bitmaps.tbl, given that groupa.pig isn't loaded until after parsing bitmaps.tbl, so far as I can tell from looking at the main function. ah well, this limits the usefulness of this, but I can still make that ILBM dumper if I'm bored enough...

edit5 (will it ever stop aaaa) hmm I guess this is what I get for charging into a untested, unmaintained, mostly useless to 99.99% of the playerbase code path. Tracing the original source code, piggy_init is called during bm_init_use_tbl, which should please the editor. D2X-Rebirth has rearchitected the loading code to load the HAM file before the PIG file, which works because the HAM file isn't reliant on PIG data until you're actually in the game, but the tablefile parser requires its presence. Given that again, this is a untested, unmaintained, almost entirely useless (I'm probably the only person since the start of the project to even attempt to run this...) code path that's not useful to 100% of players, I'm not sure how much it's worth trying to patch this, especially if it makes the normal loading route less elegant or the like. ed yet again: and despite all of that I decided to try to fix up the problems. I've gotten most of the file parsing, but there's still issues. If there's any interest in actually fixing this all up, I can open up a bugs thread about the problems I've encountered.
Reply
#14
Where would I download this?
Reply
#15
I don't have a DL link just yet, I'm still working on it. Aside from these past couple of side diversions, progress is coming along really nicely and I should hopefully have the first release done in a week or two.

So far, the HAM editor is nearly complete, I need to finish hooking up some controls in the UI and alter a few things there. The polymodel export for blender needs some tweaks, but it produces workable data just fine. A lot of the more minor editors are in a pretty sorry state, and I'm not fully sure what to do with them, but they aren't as important. I'm still heavily debating whether or not I should keep HXM editing, it's a LOT harder to manage than just toying with the HAM file itself. VHAM support should hopefully be quick to finish up, since VHAMs are a lot more convenient to manage compared to HXM files. There's a viewer for HOG, PIG, SND, and POG files, but I'm wanting to actually allow editing them at some point. They're lower priority though, since outside of POG and HOG files none of them really need to be edited.
Reply
#16
(02-27-2019, 06:00 AM)InsanityBringer Wrote: if it's going to be a perf hit, it's probably not worth checking SOBJCALL jumps at runtime, provided that there aren't any potential security issues associated with it.
I don't know how big a hit it would be. I'd prefer to take the hit once at load time, rather than on every render. I think that will not be too hard to do.
(02-27-2019, 06:00 AM)InsanityBringer Wrote: re: 8.3 filenames... wooow, heh, I haven't thought about those kinds of problems in a while, heeh... Too used to things like GZDoom where a lot of those limitations have been excised. Oops. I'm curious why I didn't get any crashes on my end. hmm
The limitation is mostly fixed. It still stores the names in the hi-scores file in the original file format, which is constrained. You probably don't have sufficient boundary checking enabled, so your build simply caused memory corruption and kept going. I enable very aggressive bounds checking when working with data I know will be ill-formed (like these models).
(02-27-2019, 06:00 AM)InsanityBringer Wrote: Also, I have one other issue I'm having trouble tracking down, the ship model has GLOW calls associated with them, and as expected it properly makes the red bits glow, but it also seems to be affecting other polygons. My assumption was that it only affected one face, but I might have misinterpreted the interpreter code regarding GLOW calls. hmm.
Sorry, no comment on this one yet. I'd need to step through it.
(02-27-2019, 06:00 AM)InsanityBringer Wrote: EDIT2: hmm, I'm getting a lot of segfaults trying to parse something the old dos descent 1 editor build parses just fine. Presumably Rebirth can still handle descent 1 1.0's BITMAPS.BIN file, so I'll look to see what's different and try some debugging to figure out what's going on.
EDIT3: ugh this is gonna be one of those wall of edits, GDB is pointing the segfault at the atof function. Interesting. Some tiny edits to the file allow it to be parsed (I had to remove all \ newline escapes and monkey around with the AI defintions before it would go. I still have the original file for comparison and further debugging, of course)
I test for crashes regularly, but if it is a data-dependent crash, I may not be testing with the right data files to provoke it. I'd like to eliminate all reproducible crash bugs, so if you can provide a stacktrace, I'll look at this too.
(02-27-2019, 06:00 AM)InsanityBringer Wrote: edit5 (will it ever stop aaaa) hmm I guess this is what I get for charging into a untested, unmaintained, mostly useless to 99.99% of the playerbase code path. Tracing the original source code, piggy_init is called during bm_init_use_tbl, which should please the editor. D2X-Rebirth has rearchitected the loading code to load the HAM file before the PIG file, which works because the HAM file isn't reliant on PIG data until you're actually in the game, but the tablefile parser requires its presence. Given that again, this is a untested, unmaintained, almost entirely useless (I'm probably the only person since the start of the project to even attempt to run this...) code path that's not useful to 100% of players, I'm not sure how much it's worth trying to patch this, especially if it makes the normal loading route less elegant or the like. ed yet again: and despite all of that I decided to try to fix up the problems. I've gotten most of the file parsing, but there's still issues. If there's any interest in actually fixing this all up, I can open up a bugs thread about the problems I've encountered.
Yes, please report details and any fixes you have found. The editor runs for me with the data files I use to play/test.
Reply
#17
Since something I've wanted to do with this tool is allow for the insertion of new elements and potentially deletion, for completely customized games or something, I decided to implement a fancy reference counting system, so that references can be automatically updated. Honestly at the end of the day, I feel doing this was a waste of time, with there probably being a less than 1% chance that any of these options are actually ever used, because problems can occur if you remove elements (there are hardcoded IDs), and you can only add so many elements before static limits in the engine are hit. I'll probably leave the capability around, since adding might be useful (there's room to spare for extra weapons and robots, so you wouldn't have to load a V-HAM file if you wanted more robots) but deleting will probably be limited to only deleting new elements past the original counts, you can't lower it than what the original HAM file has.

While this diversion has been a waste of time ultimately, it has given me one interesting feature:
[Image: M1SfgaI.png]
finally i can actually do anything with the otherwise useless log window that's been in this project since I've started it.

Thinking back on it some, I remember when I originally started this project eons ago, this was a project goal from the start, the ability to completely customize the HAM file contents, for potentially modified engines, but in practice I'm likely to abandon that overall idea. The idea of modifying the engine for projects seems somewhat unlikely, and in the future if DXX-Rebirth becomes more flexible it seems like it'd be better to modify the tool then, when the feature actually exists and isn't just a scrap in my head. This design goal had a lot of annoying fallout for a while, until recently all things in the UI were referred to by ID when I could have just used combo boxes instead. This was because I didn't want to actually manage those combo boxes.

That said, the reference counting system should have one use, since elements in V-HAMs can be added and removed freely, it's important that I'm able to track what's being used so you don't delete a weapon that's being used by a new robot.
Reply
#18
Well, I'm officially ticked off with myself.

Though it's lower priority, and the HAM stuff is going to get finished up before I even start touching this, I kinda want to write a new level editor. A minor dream I've had was to implement "decorative" objects in Descent without having to make a silent immobile no weapon robot or something like that. Descent has always had support for these objects, as "clutter" objects, but neither Descent 1 or 2 have officially used them, and no editor has officially allowed you to place them. I have noticed it was possible to define the types in Descent 1 using special fields in its pig file, but those didn't exist in Descent 2 to the best of my knowledge. These fields serve as an object registry to help map object ids to the correct data.

Except these fields aren't relevant at all, at least for clutter types. For clutter types all those registry fields are there for are to assist parallax's own editor, beyond that the thing seems to just accept whatever fields you have spit out into the level data. So, by hex-editing a level to change a robot into a clutter type, you can just drop a clutter object into a level with no issue whatsoever. Well, at least none that I can notice so far...

Agh, I'm so frustrated, this is the kinda cool thing that would have been cool to find two decades ago, not here in the year 2019, but what can you do...

EDIT: So I guess this means I've now walked two unmaintained, untested paths of code at this point. Given that there's no reason for this to be unsupported, I'll report any bugs that come up in my testing. Given that parallax themselves never used clutter types ever, I wouldn't be surprised if there are some anomalies. So far though things seem to work, you can bump them fine, you can shoot them, they render correctly. I need to play around with the render flags, it might be possible to do fun things like make them render as sprites instead of models. It's basically an object type you can do whatever you want with, so long as it doesn't involve making it move because they only have physics interactions defined between other objects and will crash the moment they touch a wall....
Reply
#19
Let's consider "crashing when touching a wall" as the first bug report for them. Wink I'm guessing the crash is
Code:
        Error( "Unhandled object type hit wall in collide.c\n" );
If so, adding another case statement to the switch to ignore the collision is easy. It might need some token handling to bounce it off the wall instead, but since powerups can ricochet off walls and have no handling here, I think that is probably already handled elsewhere.

For what it's worth, I've seen the OBJ_CLUTTER handling in the code many times, but never looked into what it was or whether it was used.
Reply
#20
I wasn't able to look at the Suzanne crash yet, but I did have time to draw up a patch for extended sub-object calls. I don't have any test POF files to validate this, so it is compile-tested only so far. I still hope to address that crash soon.

Usage:
  • Patch latest github/master with the below patch. (For best results, you may wish to start a reply-with-quote of my post, then copy the text from the quote box, rather than trying to copy the text directly from a reading view of my post. I think the forum preserves tabs in reply mode, but not in view mode.)
  • In your model, use opcode 9 for subcall32, instead of opcode 6 (legacy subcall16). Use a 32-bit displacement in the IDTA block. As you noted above, there is sufficient room for it as-is, so the new subcall32 record is the same size as the legacy subcall16 record. Where the old format ignored the final 2 bytes of the record, the new format treats those as the most significant 2 bytes of the 4 byte displacement.
If this works well for you, I can finish the big-endian/aligned-endian parts and commit it to source control.

Legacy subcall16 remains supported to keep retail models working, so you can use subcall16 in new models if they do not require the longer-range displacement. You can even mix subcall16 with subcall32 in the same model, but I don't think that would be very useful. Rebirth builds that accept subcall32 do not benefit from using subcall16, and Rebirth builds that lack subcall32 will reject the entire model.
Code:
diff --git a/similar/3d/interp.cpp b/similar/3d/interp.cpp
index b971ea474..dbb256795 100644
--- a/similar/3d/interp.cpp
+++ b/similar/3d/interp.cpp
@@ -35,6 +35,7 @@ constexpr std::integral_constant<unsigned, 5> OP_RODBM{};   //rod bitmap
constexpr std::integral_constant<unsigned, 6> OP_SUBCALL{};   //call a subobject
constexpr std::integral_constant<unsigned, 7> OP_DEFP_START{};   //defpoints with start
constexpr std::integral_constant<unsigned, 8> OP_GLOW{};   //glow value for next poly
+constexpr std::integral_constant<unsigned, 9> OP_SUBCALL32{};   //call a subobject, 32-bit displacement

#if DXX_USE_EDITOR
int g3d_interp_outline;
@@ -187,6 +188,9 @@ public:
        (void)p;
#endif
    }
+    void op_subcall32(const uint8_t *const)
+    {
+    }
};

class g3_interpreter_draw_base
@@ -256,12 +260,20 @@ protected:
        };
        g3_draw_rod_tmap(canvas, *model_bitmaps[w(p + 2)], rod_bot_p, w(p + 16), rod_top_p, w(p + 32), rodbm_light);
    }
-    void op_subcall(const uint8_t *const p, const glow_values_t *const glow_values)
+    void op_subcall_internal(const uint8_t *const p, const glow_values_t *const glow_values, const uint8_t *const subp)
    {
        g3_start_instance_angles(*vp(p + 4), anim_angles ? anim_angles[w(p + 2)] : zero_angles);
-        g3_draw_polygon_model(model_bitmaps, Interp_point_list, canvas, anim_angles, model_light, glow_values, p + w(p + 16));
+        g3_draw_polygon_model(model_bitmaps, Interp_point_list, canvas, anim_angles, model_light, glow_values, subp);
        g3_done_instance();
    }
+    void op_subcall(const uint8_t *const p, const glow_values_t *const glow_values)
+    {
+        op_subcall_internal(p, glow_values, p + w(p + 16));
+    }
+    void op_subcall32(const uint8_t *const p, const glow_values_t *const glow_values)
+    {
+        op_subcall_internal(p, glow_values, p + *reinterpret_cast<const int32_t *>(p + 16));
+    }
};

class g3_draw_polygon_model_state :
@@ -348,6 +360,10 @@ public:
    {
        g3_interpreter_draw_base::op_subcall(p, glow_values);
    }
+    void op_subcall32(const uint8_t *const p)
+    {
+        g3_interpreter_draw_base::op_subcall32(p, glow_values);
+    }
    void op_glow(const uint8_t *const p)
    {
        glow_num = w(p+2);
@@ -412,6 +428,10 @@ public:
    {
        g3_interpreter_draw_base::op_subcall(p, glow_values);
    }
+    void op_subcall32(const uint8_t *const p)
+    {
+        g3_interpreter_draw_base::op_subcall32(p, glow_values);
+    }
};

class init_model_sub_state :
@@ -453,6 +473,10 @@ public:
    {
        highest_texture_num = init_model_sub(p+w(p+16), highest_texture_num);
    }
+    void op_subcall32(uint8_t *const p)
+    {
+        highest_texture_num = init_model_sub(p + *reinterpret_cast<const int32_t *>(p + 16), highest_texture_num);
+    }
};

constexpr const glow_values_t *g3_draw_morphing_model_state::glow_values;
@@ -503,6 +527,11 @@ static std::size_t dispatch_polymodel_op(const P p, State &state, const uint_fas
            state.op_subcall(p);
            return record_size;
        }
+        case OP_SUBCALL32: {
+            const std::size_t record_size = 20;
+            state.op_subcall32(p);
+            return record_size;
+        }
        case OP_GLOW: {
            const std::size_t record_size = 4;
            state.op_glow(p);
@@ -632,6 +661,7 @@ public:
        short_swap(wp(p+16));
        swap_polygon_model_data(p + w(p+16));
    }
+    /* op_subcall32 not implemented yet */
    static void op_glow(uint8_t *const p)
    {
        short_swap(wp(p + 2));
@@ -700,6 +730,7 @@ public:
    {
        add_chunk(p, p - data + new_data, 16, list, no);
    }
+    /* op_subcall32 not implemented yet */
};

}
Reply


Forum Jump:


Users browsing this thread: 5 Guest(s)