Also it - assuming i got it right - moves small ship, big ship and huge ship flags from ship info to ship type info and makes then along the bomber turret thing as an option for the objecttypes.tbl.
Woah, woah,
woah.
The entire point of objecttypes.tbl was to get rid of those flags. They cause a lot of hardcoded behavior that needs to be broken down into individual options so that mods can specify it individually, and so that it's clear what's actually happening. I broke a lot of those size-dependent options off of the flags and moved them into objecttypes.tbl. From a design perspective, I'm not sure if it's a step forwards or a step backwards. In the short term, it gives the modders a little more choice as they can select from three super-shiptypes.
In the long term, it risks halting development of objecttypes.tbl and permanently retarding its growth, possibly stopping it entirely. Frankly, it scares me. By making those flags accessible to modders rather than simply an internal code thing, you would force every single version of the SCP to continue to support those flags. That means that every single toggle that's in there now for those flags would have to stay there. If somebody ever gets around to making those options explicit in objecttypes.tbl, every single option, every single value, would have to have code implemented to handle precedence for the flag.
For example, what happens if somebody explicitly specifies a value in one modular table, and then a modular table after that specifies a "big ship" flag? Do you override the explicitly specified variable in the first modular table with the implicit specification in the second modular table? That's the easiest method from a coding POV, but it's a confusing result for the modder.
Any time you make something from the codebase explicit in a table or other modder-accessible thing, you force every single version of the SCP to be consistent with the effects of that. You can't just port variables to a table without any thought to how it's going to effect things if somebody else wants to do something more than you, or improve on the work you've done, unless you just don't care or don't want to allow for that possibility. It's a very daunting task, but it's generally a lot better to provide a lot of specific options rather than very vague ones. If you have an option like "Max Visible Fog Distance", you know what the modder wants and you can play around with ways to do that. You can add whatever gimmicks you need to account for new materials and so on, because you know that the modder wants the ship to be visible at that distance. If the modder just says that the ship is a "big ship", there's no inherent implication of how far away the ship should be visible in fog. You also don't know if the modder is using the flag because they think the ship is a big ship, or because one of the options is so important that they're using the flag inaccurately to get one specific effect.
There's also the matter of move stuff from ship_info to ship_type_info. That should, generally speaking, never
ever be done. In this particular case, it doesn't really affect anything since the options were dependent on ship types anyway. However, in general, all this does is reduce modders' choice in what ships should do. In many cases, options that were originally ship type dependent should be moved to ships.tbl because somebody might want to make them different for ships of the same class. Remember that ships of the same class may not necessarily be of the same species, so the technology behind them may be totally different. I've heard the objection that it creates more work for table modders, but IMHO that's not true in the long run. When you factor in managing precedence not just between modular tables, but also between different types of tables, it's a lot better to proactively avoid the precedence hell. It's also a lot easier to code the feature once than to have to recode it later on, when somebody wants the option on a per-shipclass basis, and also take into account that you've already coded it once before.
Speaking less formally, I have little patience for people who are willing to risk all of that rather than have to paste one line in a table 15-16 times.
I wish any coding were that easy.
And, like I said, it goes against the original intent of ship types, which was to get rid of all those designations which make sense for the Freespace universe, using the original game's ships, and make it into something that mods could use to start from scratch to define their own ship types with their own characteristics.
Notes on the ImplementationThree different boolean options are used to identify a ship as big, small, or huge. When you're working with a set of a type of designation, it's generally better to just make that a single variable. There's no reason to have three booleans, it wastes memory. Also, a ship can't be small and huge at the same time, at least not with the conventional english definition of those terms.
Several places you use:
Ship_types[Ship_info[Ships[objp->instance].ship_info_index].class_type].ship_boolsFor me this is a red flag, because there are three indices going on. I think I did this before, but it wasn't really a great idea then. It would be more appropriate to make a function like this:
ship_type_info *ship_get_type(object *objp)
{
Assert(objp != NULL);
Assert(objp->type == OBJ_SHIP);
Assert(objp->instance > -1);
Assert(Ships[objp->instance].ship_info_index > -1);
Assert(Ship_info[Ships[objp->instance].ship_info_index].class_type > -1);
return &Ship_types[Ship_types[Ship_info[Ships[objp->instance].ship_info_index].class_type];
}which cleans up the code everywhere that the indexing is made, and lets you check all of those assumptions automatically in a debug build.
I would also change "Use small ship turrets" into a "Turrets prioritize ship target" option, or something shorter along those lines.
Also, at certain points you changed a "enemy_sip_flags" to ship type bools. For that kind of thing, the variable name should be changed, as it's no longer ship info flags. Somebody else might get confused by that bit of code and think that ship_info flags were being referenced, put in some checks for SIF_ flags, and get unpredictable behavior.
Sorry that got pretty long.
