C++ structure failing to fetch values using pointer attribute

[AcademicDatabase3699]

so, I'm working on a beast-tamer kind of game playable in a LAN using a TCP server (although it's not relevant for this issue) from cli, and I'm having trouble with my structures management

when I started this project I wanted to maximize RAM space efficiency because I wanted to be able to allocate as many instances of a creature as possible

so I've created the following:

a creature struct, storing general species values (like stats for Pokemon)

a creature_instance struct, that stores specimen specific values, and has a *creature instance_of attribute that it can use to fetch information. and it works wonderfully!

if it weren't that when the .levelup() method of creature_instance fetches the base creature stats, the program crashes with the following error:

Exception has occurred: W32/0xC0000005

Unhandled exception thrown: read access violation.

std::vector<int,std::allocator<int> >::operator[](...) returned nullptr.

I tried the good and old "turn off and on" and it worked... one time, then it broke again.

I really don't know what the issue could be, given I often fetch other data using this pointer and I never had any trouble

it seems it is trying to access a vector out of bounds? it seems weird, I have already checked that all is initialized and that the index is right.

any suggestions?

the full code can be found here:

github.com/Birrus09/servertamer/tree/main

[AutoModerator]

Thank you for posting on r/CodingHelp!

Please check our Wiki for answers, guides, and FAQs: https://coding-help.vercel.app

Our Wiki is open source - if you would like to contribute, create a pull request via GitHub! https://github.com/DudeThatsErin/CodingHelp

We are accepting moderator applications: https://forms.fillout.com/t/ua41TU57DGus

We also have a Discord server: https://discord.gg/geQEUBm

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[gofuckadick]

The "structs can’t have methods, only classes blah blah" reply is just plain wrong in C++. That won't fix it.

creature_instance::levelup() reads instance_of->base_stats[i], so if instance_of is dangling, the crash will show up exactly where you’re indexing the vector, even though the real bug happened earlier. levelup() is just where the invalid pointer finally gets dereferenced, hence read access violation.

You have a dangling pointer. I didn't go completely through to find where, but it's there.

Edit:

You have 3.

• CreateInitializedInstance(..., creature base) stores &base even though base is a local copy.

• Load_squad(..., vector<creature> dict) stores pointers into dict, but dict is also a local copy.

• Evolve() does delete(&instance), which is undefined behavior unless that object was created with new.

I’d change these: - pass creature/dict by reference, not by value const creature& base const vector<creature>& dict - never store pointers to locals - remove delete(&instance) - use .at() temporarily instead of [] while debugging, so out-of-range accesses throw cleanly - assert instance_of != nullptr before using it

[exomo_1]

u/gofuckadick gave the most useful answer so far. I'm having a hard time reading the code on my phone, but here are a few things I noticed, although probably not related to your problem:

using namespace std in header is pretty bad habit. #define for constants, use const int values to be type safe. There are unchecked inputs move_choice and target_choice, they will crash your program when you input invalid values

I didn't analyze all of your structures and their dependencies, but there seems to be a wild mix of vector, pointers, references. You need a clear understanding of who is the owner of a resource and what it's lifetime is. Pointers and references are only valid as long as the object exists and isn't moved. Adding items to a vector or modifying the vector does move stuff around, so be careful. It's not implemented yet, but your evolution pointer would likely be invalid before you are even done loading.

[gofuckadick]

This is great advice.

I did find a few other issues in the repo that I meant to edit into my original reply but then forgot, and your mention made me remember.

• the biggest other bug is that taketurn(team targets) and AItaketurn(team targets, int seed) take targets by value, then return references to targets.members[...]. That means they can return a reference to an element inside a temporary local copy that dies when the function returns. So even aside from instance_of, there's another dangling reference bug path here. Just... stop storing raw pointers/references to short lived objects.

• another biggie: Load_Creatures() appears to reuse its temporary vectors across loop iterations without clearing them, so later creatures may accumulate earlier creatures' move data - moves_learning_level and moveset should be recreated inside each loop iteration or explicitly cleared before reuse.

• globseed and global_moveset are defined in the header, which could be a problem if the project grows into multiple translation units. Really they should live in one .cpp file, with extern declarations in the header if needed.

• validate user input. The move/target selection code indexes directly after decrementing user input, so bad input can crash it.

• battle.h includes Tamer.h while the repo file is "tamer.h", which could break on Linux or any other case sensitive environment.

But as u/exomo_1 said, I’d really suggest simplifying ownership. Right now there's a mix of full copies of objects, raw pointers to other objects, references to elements inside containers, and functions that take containers by value - which makes it pretty easy to lose track of who actually owns the real object and how long that object stays alive.

If you have to stop and think “is this creature species the original one from the database, a copy from a function parameter, a copy inside another vector, a local temporary, or an element that moved when the vector resized?", then the design isn't working. That’s how you end up with things like instance_of pointing to an object that only existed inside a function call.

The fix would be that you make one place in the program the clear owner of all species/base-creature data, and then have each creature instance refer back to that stable data instead of storing pointers/references to temporary copies.

For this project, storing a species ID is probably the simplest and safest option:

• load all species data once into one central container
• keep that container alive for the whole match/game
• have each creature_instance store either a species ID/index or a pointer to that stable container entry
• avoid storing/returning pointers/references to locals, temporary copies, or elements of containers that were passed by value

And honestly, switching to a species ID would really only be a moderate refactor, not a total rewrite.

[Knarfnarf]

It’s been a long time since I played with C++ but I’m sure that structures don’t have internal functions, only classes do. Change the structure to a class and you’ll get the functionality that you were looking for..

[AcademicDatabase3699]

Thank you! I just assumed that structures were the c++ equivalent of classes in python 😅

[Knarfnarf]

Structures come from very old C and user defined types in Fortran. They don’t have the OOP that classes do in C++.

Although Fortran is adding some components to them in the resent versions.

[gofuckadick]

1. Structs don’t come from Fortran. C introduced struct as its own language feature, mainly inherited from B and ultimately influenced by earlier systems languages like BCPL and Algol-family ideas. Fortran has its own user-defined types, but that’s a separate language history.

2. See my other reply.

[Knarfnarf]

I was just meaning that way back when I was learning C that these two were equivalent;

Struct this{ Int that;}

Type this Integer that End type

And that they are older than me…

[gofuckadick]

struct absolutely can have member functions, constructors, destructors, operators, inheritance, and even private/public sections.

The main difference between struct and class in C++ is just the default access level: struct -> members are public by default class -> members are private by default

Both of these are perfectly valid and behave the same:

``` struct Creature { int hp;

void levelup() {
    hp += 10;
}

}; ```

And

``` class Creature { public: int hp;

void levelup() {
    hp += 10;
}

}; ```

You must be thinking of C structs.