Err… that's a perfectly fine place to declare the variable.

Indeed. That's why I love it.

But it's not really a perfectly fine place to put it in C considering it only works in C++ compilers. :P

It's been accepted into more recent versions of C, right? Definitely not ANSI C, but …

It doesn't work in gcc without g++ option. I think it's extended to work in other compilers.

additionally: Which is one of the reasons I don't like having to conform to things on the project that compile in gcc as well as g++. Lexical stuffs like this.

Actually, it does work in C99 mode. –std=c99.

cat test.c


#include <stdlib.h>
#include <stdio.h>

int main()
{
    for (int i = 0; i < 10; i++) {
        printf("%d\n", i);
    }

    return 0;
}

gcc -std=c99 test.c

./a.out
0
1
2
3
4
5
6
7
8
9

Yes, that's all well and good for truly temporary variables, such as loop counters.

It's a bit uglier when people stuff variables into loops and then later cut/paste to another loop and wonder why the value doesn't get preserved between the two…

Yes it's sloppy, and yes the coder should know better…. but they don't. :) Thus I try to minimize such things to avoid confusing people who are using this plate of spaghetti as their introduction to C course.

You think that making local loop variables is a dangerous thing to do? I kind of think on the contrary that it's extremely good practice because it enforces the fact that something that is conceptually local to the loop is in fact only accessible from that loop. I try to avoid making decisions based on what mistakes a hypothetical sloppy coder will do, because that means you end up punishing yourself for somebody else's potential mistakes.

I personally don't think that declaring a variable inside of the loop like that is sloppy.
If it's a loop counter variable that seems to make sense and encapsulate it from the rest of the code. (Where presumably it shouldn't be used.)

Additionally: We should just only use a global "int i". Then we never have to declare it again in any function. I'm that lazy. ;)

That took longer than I thought… But finally finished replacing all of the ch->next and ch->next_in_room lists with the new style lists and iterators.

It solves some of the other problems that were fixed with hacks before. Like being able to free the current node of iteration before advancing iteration. Not having to worry if the current node of iteration gets freed in a function call. (Like the damage functions that don't guarentee the pointer is valid after the call. The iterator knows when the item is no longer in the list and replaces it with a empty variable.) On the last count, still likely to break stuff–but not corrupt other memory with a freed pointer.

By new-style do you mean C++ STL or something else?

We'll be using STL stuff when we actually work on Fire, but for the Ice branch we're using a generic container system wrapped around functions and type defines per common item that is contained. It also uses an iterator with our own needs in mind.

Here's some example code.

// clist holds CHAR_DATA *
CHAR_LIST  clist;
init_char_list(&clist); // initialize our list.

while ( we have data to put in list)  {
    ch = cast_data();
    char_to_list(&clist, ch); // defaults to push_front (can also call char_to_list_back() to push_back)
}

// so we have a list with a certain number of elements in it populated.

// Iterate over the list and print each "name" field from the char_data in the list.

for(ITERATOR it = get_char_iterator(&clist);it;it = next_char_iterator(it)) 
    printf("%s ", ref_char_iterator(it)->name); // prints name.

// You can "peek" at any element without popping it from the list with this function
peek_char_list(&clist, 1); // returns the CHAR_DATA *  at the first position.
peek_char_list_back(&clist, 1); // returns the CHAR_DATA * at the first position from the back of the list.
// Note, these wouldn't remove it.

// Now let's pop the top element from the list and print its name.

if (count_char_list(&clist) > 1) // only if the list actually contains anything.
    printf("%s ", pop_char_list(&clist)->name); // removes the first node and returns the data it had.

// If we have nodes left then let's iterate through them, freeing them, and preserving the correct list.

for(ITERATOR it = get_char_iterator(&clist);it;it = next_char_iterator(it)) {
    printf("%s ", ref_char_iterator(it)->name); // prints name;
    remove_char_list(&clist, it); // Remove the element at it, doesn't matter what gets freed or removed.
    // Continue on even though this node isn't valid any more.
}

free_char_list(&clist); // clean up the list if it's being destroyed.

There's more stuff that I have it set up to do. Basically the idea is being able to mock some of the features the STL has for our C version if we try to keep them both up to date with each other. Right now it can use ITERATOR or REVERSE_ITERATOR or CONST_ITERATOR, apply algorithms via function casting.

I've got only got GENERIC_LIST, CHAR_LIST, INT_LIST, and STRING_LIST right now but the plan is to have one for every time of commonly used data type in the mud. And if someone just wants to use a list without having to define stuff they can use GENERIC_LIST, even though that would require knowing datatypes and casting to void pointers. (Although it has some safety since it requires you to acknowledge the type everytime you call it.)

example: generic_to_list(&glist, (void*)some_data, "CHAR_DATA");

No, not perfect. No, not as efficient.
Yes, more robust. Yes, faster development.

Another thought is that if we do this maybe people won't have such a hard idea transitioning to C++ style containers and iterators in the future, since obviously it's a better solution.

Are you copying the whole list on iterator creation in order to get safety?

Nope. It uses an underlying generic_iterator that is registered to the core generic_list.

Then the generic_list calls and interacts with the registered iterator appropriately to establish "direction" during runtime. This allows it to be (possibly) thread safe as well as safe to function calls during the iteration.

The actual iterator layer simply works with generic iterator and updates to the correct values if need be when next_char_iterator() or prev_char_iterator() is used.

Also, even though this is intentional, if an element is added to the grey side of the list then it will be not be ignored.
If the element is added to the already colored side then it will be.

This is dependant on if it's a reverse or normal iterator. On a reverse the colored list is the back to front, on the normal it's front to back.

If elements are reordered but the node you are currently iterated at is not deleted then you will probably have to make sure you are reordering it based on some type of logic.

Such as, iterating to a certain string in a string list. Then applying a sort algorithm to sort by alpha. Whatever node you are iterated to–At that point everything in the list that is before that node in sort by alpha would be in the colored list. Anything else would be in the grey list. (Even though you may have iterated some of the grey already.)

That's also intentional.

Regardless, we think it's a step above the stock rom system for C. That's why I developed it.

Continuing my quest to remove 1990's hacks from RaM, I was looking to get rid of the recycle system (now that I've isolated it a bit), and ran across a comment that gave me pause.



The problem here…. how much of ROM's code actually believes that false assumption?

I say false, because you won't ever know if what you looked at is what you wanted. Sure, since it isn't freed, it won't cause a seg-fault, but if it was stuck on the free list and then reallocated to someone else, it may be totaly different data.

So my question is… how much of ROM/Merc/Envy will break if alloc_perm() were replaced with malloc()? Seriously, it makes sense for the shared string system to use this… but anything else is asking for trouble.

Continuing to peruse the gsn silliness, there seems to be no rhyme or reason for the way spells and skill are allocated.

Things that appear to be true:

All entries in skill_table have a spell_fun, however all skills point to the function spell_null(), which, as you'd expect, does nothing. The function itself seems to only be used as a failsafe check in do_cast() which prevents skills from being "cast".

All spell entries have NULL pgsn entries.

While both spells and skills have slot numbers, there doesn't seem to be any overlap in their values – but there also doesn't seem to be anything preventing that from happening! The slot numbers for spells are defined in the table definition (in no particular order), but the slots for skills are assigned by a routine during boot. AFAICT, there is nothing in place to prevent you from defining 300 skills and stomping over the first chunk of spells.

One way to solve the problem is to give the skills fixed slot numbers in the table definition, and remove the assignment loop along with all the gsn_ variables. You'd then treat them just like spells, using SKILL_FOO #defines to refer to them, rather than gsn_foo variables.

I want to look into how spells/skills are used with objects and/or rooms too. I know how to make a "wand of fireball", but can you make a "wand of haggle"? Does the code distinguish between spells and skills at that level, at all?

HAHAHA – Of course, after posting I find an exception.

"blindness" is both a spell and a skill, and so it has a gsn_ variable and a spell_ function. Even worse, it will have SLOT(4), but the gsn_ will be some other number! Effectively, two seperate versions of the same thing that share all the same values. Naughty!

Slots aren't used in rom

It looks like the only place they're referenced is in converting from "slot number" in loading objects (that have spell effects) to skill index numbers.

It looks like player save files already save skills by name. I see no reason not to do that for area files as well. If the skill table is sorted by name (one call to qsort() at boot time, another if OLC modifies that table), a simple binary search would make even the skill numbers irrelevant.

I'll poke around a bit more, but it looks like it should be a fairly clean transition.

I always thought that the slot number system was a kind of weird solution to a problem that could have been solved by using names or something like that – or if you want to be able to change names, give skills a unique named identifier, and use that as the primary skill designator, and make names be an attribute like anything else.
Well, neither here nor there at the moment…