Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropping the hardcoded "corpse" item, generating per-creature-type corpses #20069

Closed
Coolthulhu opened this issue Jan 22, 2017 · 18 comments

Comments

Projects
None yet
5 participants
@Coolthulhu
Copy link
Contributor

commented Jan 22, 2017

The "corpse" item is limiting and implemented in ugly, hardcoded way.

Instead, we could have all creature types generate their own type of corpse, which would then be generated from a template on finalization.
Corpse's ID could be either required to be named explicitly, in which case the corpse item should have a definition somewhere, or more conveniently it could be generated from creature's id instead. For example, by prepending "corpse_" to it.

This would allow:

  • Adjustments to corpses, for example changing their weight or making glowy creatures stay that way after death
  • Recipes that use corpses
  • Dropping the ugly hardcoded corpse code
@keyspace

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2017

Not familiar with corpses at all. Took a glance.

Seems that there's an item with id corpse (in data/json/items/generic.json), corresponding to item with typeId() == "corpse" that has an associated mtype corpse.

One must not pass a null pointer when making a corpse item, but an item can return a null pointer if it's not a corpse... (EDIT: This about item::{get,set}_mtype().)


What would the benefits of having separate corpse_* ids be? As compared to, say, the naive monster just having an is_corpse boolean. Perhaps:

  • Don't process corpses as active monsters while loaded in reality bubble.
  • Don't process corpses as part of a horde while out of bubble.

Arguably possible in both cases:

  • Possibly process corpses to revive while out of bubble.
  • Zombification paths, e.g. mon_moose -> ? -> mon_zoose.

EDIT: Anyway, yeah, making a corpse part of a creature would repeat the current situation where a corpse is part of item (see below).


EDIT (2 times): expand lists

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2017

What would the benefits of having separate corpse_* ids be? As compared to, say, the naive monster just having an is_corpse boolean.

Corpses have to be items, they can't be creatures. They can "store" a creature for stuff like proper revival instead of re-creation, but they can't be just flagged creatures.

What we have now is one "corpse" object which has very little information in it, but instead recalculates everything on demand, using dead creature's monster type. This is very inflexible for purposes of overrides and also makes the monster type responsible for parameters of the corpse that are only related to being an item.

@keyspace

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2017

Yup, that's what I understood. Concise explanation, thanks.

Would it be prudent to have a corpse class, derived from item?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2017

Yes.

There would most likely be multiple such base classes:

  • One "the most base", defining tags that all dead critters need
  • One for critters made of flesh
  • One for robots
  • One for plants

etc.

Such hierarchical structures are useful for type checking: "is it a corpse?", "is it a zombie corpse?", "is it a zombie corpse that has cbms?" etc.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 23, 2017

Please don't plan on subclassing item, there are very good reasons for the islot idiom over a subclassing approach.

In general though, different itypes for different creature corpses is a great idea, I'm not so sure about auto-generating them though, especially if that means adding more data to creature definitions, it's a bit of a separation of concern issue, and we may well not want a different itype_id for every creature type, some might be shared between monsters.

A development approach suggestion, please consider implementing this piece wise, i.e. transition the simplest monster possible to this new system, allowing other corpses to continue using the old system, repeat until the system covers all the extant functionality. Novel features can be added as separate PRs once the corpses they target are transitioned.

Bottom line, I don't want an "overhaul corpse generation" pr before 0.D, either proceed very incrementally or expect to keep your branch out of mainline until the next stable.

@Chezzo

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2017

Would this let me make different tiles for pulped corpses vs unpulped corpses?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 23, 2017

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Feb 6, 2017

Please don't plan on subclassing item, there are very good reasons for the islot idiom over a subclassing approach.

Such as?
I see multiple games doing subclass tree to a good effect: Caves of Qud and Rimworld, for example.
It is very maintainable and could be used for great effect with mods if deferring was forced (currently it is optional). Say, you want to add "insect blood" harvest to all insect corpses - in "flat" system it means overriding all of them manually, in "tree" one it means altering one definition.

I don't get the aversion to nesting.

we may well not want a different itype_id for every creature type, some might be shared between monsters

In this case we could have a field like "corpse_item". This would allow, say, soldier zombies degrading into tough zombie corpses when dead and thus stripped of gear.

A development approach suggestion, please consider implementing this piece wise, i.e. transition the simplest monster possible to this new system, allowing other corpses to continue using the old system, repeat until the system covers all the extant functionality.

I'd probably start with allowing items to arbitrarily qualify as corpses based on something other than ID.

Bottom line, I don't want an "overhaul corpse generation" pr before 0.D, either proceed very incrementally or expect to keep your branch out of mainline until the next stable.

Sure. It's mostly brainstorming at the moment.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Feb 7, 2017

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Feb 7, 2017

Slots allow new combinations of types to be created by declarations in data, a subclassing approach would require you to enumerate and define every combination of subtypes you want to use in c++.

I don't think I follow.
Can you provide an example of how you wouldn't want json-coded corpses to look like and example of how they should?

@bdgackle

This comment has been minimized.

Copy link
Contributor

commented Feb 8, 2017

@coolthulu, I think I can speak to the notion of "Sub classing non-abstract bases" being a bad idea. We can use the example you gave above of having a 'base' with all the dead thing properties, and then having subclasses that flesh out the robots/plants/flesh critters.

With a subclass tree, you have to explicitly define the properties of a class in C++ when you declare the class.

The issue here is that a given critter is now hard coded to have whatever data it has. As a simplistic example, if you wanted to create a 'cyborg' creature, you'd need to explicitly inheret from the robot and flesh creature subclasses.

If, instead, you create a flat list of structures, which in this case would be:

  1. Data for all dead things
  2. Data for general robot things
  3. Data for dead plant things
  4. Data for dead flesh things

You could then have code that expects to handle just one of the above (say code for generating meat drops from a corpse, that doesn't depend on other common data). Now, you can define the corpse of, eg, a moose as having the 'general dead' data as well as the 'flesh dead' data. A hypothetical cyber-moose would additionally have the "robot things" component. An item's included data components can be defined via a series of flags.

It is conceptually very similar to using a class hierarchy in that you are factoring out common data blocks to avoid reuse, but there are a couple of key differences that are useful:

  1. You can now use data (in the form of a bunch of flags) to define which components a class includes, rather than C++ inheritence mechanisms. This allows for the item definitions to be pushed further up into JSON files.

  2. You can define totally abstract interfaces for the various component types, which makes insertion of mocks for unit testing a heck of a lot easier.

  3. Code dealing with one component can be oblivious of everything else in the hierarchy. You can decouple your 'meat drop' code from general corpse code entirely. Again, this has benefits in reducing conceptual load as well as insertion of unit tests.

I think that moving toward this sort of approach might help with refactoring of the huge disorganized classes that you recently mentioned in another issue.

As a side note, do you have a source for the use of object hierarchies in RimWorld? That strikes me as a game where the entity-component type approach would have made more sense. If they provided some reasoning for their approach I'd be interested to read about it; might be a novel point of view.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2017

Regarding Rimworld: I didn't see any rationale, just looked through the data bundled with the game.
Though it was about abstract structures, so not really the same thing.
Base item properties are defined in one object, then plant matter derives from said object, then individual plant items derive from the generic plant matter one.

The example I gave - with corpses - was also mostly about abstracts, except that some of them would have trivial instances - zombie corpse, for example.
Though now I get the point about subclassing. Without turning the tree into DAG and complicating the whole thing, some combinations would not be possible.

@bdgackle

This comment has been minimized.

Copy link
Contributor

commented Feb 8, 2017

If you haven't stumbled on the notion of Entity-Component-System game engines before, it's an enjoyable Google search. Not saying it is "right/wrong", but it is certainly another way of looking at organization, and one that I believe happens to work well when you are trying to create emergent behavior.

This is one of the first articles I read on the subject:
https://www.gamedev.net/resources/_/technical/game-programming/understanding-component-entity-systems-r3013

I stumbled on this basically after my own (very simple) attempt at creating a rougelike from scratch fell completely apart after a few weeks when my object model got too big to fit in my head.

@keyspace

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2017

ECS by design introduces IDs for objects, which has been discussed at length here. Granted, these do not have to be persistent between savegame loads, and should not be used for anything but one-to-one mapping of identity (different from referenced issue), but still, worth taking into account.

It also (essentially) requires a very efficient database with SELECT-like lookups. I've only taken a look at one ECS library so far, but database handling there wasn't very optimised - I'm afraid to think of what that could do performance-wise to a piece like C:DDA, where a game state could easily have a [huge number] of entities. If done correctly, though, it could allow expanding the reality bubble.

@bdgackle

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2017

To be clear, I am not proposing a full refactor of the game to an ECS. Performance implications aside, that's tough to do incrementally, and we don't have nice decoupled blocks of logic that can be carved off into "systems" either.

I was mainly referencing the idea of flattening the hierarchy to enable composition vs inheritance (which is not unique to ECS, that is just a good vehicle for communicating them in this context), and more specifically the ability to push the creature hierarchy into the JSON layer rather than the C++ inheritance system.

Thanks for the link -- I'll read through that discussion and see if any of the half-formed ideas in my head still make sense afterwards. I wasn't explicitly proposing persistent ID's, but they may be implicit in what I am thinking. If so I might be proposing something that is infeasible for reasons that have been decided in the past.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Feb 13, 2017

@keyspace

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2017

May be off-topic.

EDIT: Discussed on the forums.


It also (essentially) requires a very efficient database with SELECT-like lookups.

It does not, I think that might be common to prevent the developer from worrying about serialization, but it could also be implemented as a set of hash maps keyed by entity id.

IMO not just common but necessary, due to such systems needing to choose entities/components for handling by a system. This "choosing" is what I likened to SQL's SELECT above. And the choice of implementation greatly affects what the system could be optimised for.

If you have a lot of free time, there's this article by Adam Martin that goes C-low to outline the efficiency issues.

Gist: data processing order is different, and can be optimised for homogenous objects - e.g. components if 1-component-1-system is used, sets otherwise if there's an evident bottleneck, or even full entities if there's just a few of them... It all boils down to what can fit in the CPU cache.

Also, it's mostly relevant for threaded applications. So not at all for C:DDA ATM.

Could you expand on this? The thing keeping us from making the reality bubble larger is scaling of various algorithms that act on it, such as field of view and dynamic lighting calculation, I don't see how an ECS would have any impact.

Ah, sorry, I was in a different reality bubble (badum-tss...) and didn't think about those two. Doh!.. (Never looked at them - that's why.)

@kevingranade

This comment has been minimized.

Copy link
Member

commented Nov 25, 2017

This is more a todo item than an issue, and obviously no discussion in months.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.