Celebration / Issue with loot

[Rabiat]

So what exactly does that do? I've seen it in a lot of projects, but never used it for a simple lack of knowledge.

Option Explicit requires explicit declaration of variables. Using a variable 'x' without a previous declaration 'Dim x' gives a compile time error. If you leave the option off, the compiler will silently assume that x is a newly declared variable whenever you use it (even if 'x' was a typo, and you meant to refer to an existing variable 'y', for example).

Option Strict prevents narrowing conversions (if you assign an Integer value to a variable of type Byte, it'll give a compile time error). If you leave this off, the conversion will take place, causing implicit data loss if the Integer value is greater than 255. If you turn it on, you'll have to remove unwanted implicit conversions, or write explicit conversions using CInt, CByte, CType, etc..

Option Infer allows the compiler to guess a variable's type from a value declaration. It makes 'Dim x = 3' a valid statement, and makes the compiler assume that x is an Integer (even if you meant it to be something else). If you turn it off, you'll have to state the type of every variable at declaration ('Dim x As Integer = 3').

Having Explicit On, Strict On, and Infer Off prevents potential declaration, conversion and type casting bugs before they can take place at run time.

Another thing I've seen around, but lack the knowledge of how to use.

List(of T) is a collection of any object type. If you define a loot list and an inventory list, you could more or less write the GetItem routine like this:

Code: [Select]

Public Class Item
    Inherits Object

    Public Property Name As String
    '(...)
End Class

Public Class InventoryList
    Inherits List(Of Item)
End Class

Public Class LootList
    Inherits List(Of Item)
End Class

Public Class Player
    Inherits Object

    Private inventory As InventoryList

    Public Sub New
       inventory = New InventoryList
       inventory.Capacity = 26
       '(...)
    End

    Public Sub GetItem(ByRef loot As LootList, ByVal i As Integer)
        If i < 0 OrElse i >= loot.Count Then
           Console.Write("Loot has no item at position " & i.ToString)
        Else If inventory.Count = inventory.Capacity Then
           Console.Write("No free inventory space")
        Else
            dim itemTaken As Item
            itemTaken = loot(i)
            inventory.Add(itemTaken)
            loot.RemoveAt(i)
            Console.Write("Picked up a " & itemTaken.Name)
        End If
    End Sub
End Class


I tried doing things like Is Nothing, or = NOITEM [My empty item], but it pulls up an error, and the way I'm doing now works fine for the time being at least

'Nothing' is assigned differently than it's tested. Assignment is 'x = Nothing' and test is 'If x Is Nothing'. It's a stupid language feature. The NOITEM thing you did isn't bad at all, but I think it's unnecessary if you use some kind of List object. If you stick to using NOITEM, then assign it to the LootMap in empty locations, and change

Code: [Select]

If Not LootMap(Player.Left, Player.Top, intloop).Name = "Nothing" Thento

Code: [Select]

If Not LootMap(Player.Left, Player.Top, intloop) = NOITEM Then

So you think I should make a seperate sub or function just to write that they picked up an item? I think putting it in with the sub that handles picking up items makes perfect sense

I think you should write a separate sub, but that's because I'd define some kind of MessageDisplay class to handle all messages. I could explain why I think that's a wise design decision, but unfortunately don't have time to do so right now. Writing messages when something happens makes sense, but it may become somewhat more difficult to maintain as the source code expands. (For example, consider having to change each Console.Write statement when you decide you want the messages to be displayed in a different color.)

Thanks! I've used vb6 a lot, but that's pretty outdated. And I haven't programmed period in a while either, so I figured starting a basic roguelike and improving on it as I go would be a good way to oil the gears, if you will.

VB.NET is quite different from VB6. It's more solidly object oriented, and of course it uses the .NET library. If you're not familiar with .NET, I'd suggest you read up on it. It's huge, both in its capabilities and its complexity, so it takes some time to find your way around, but after that it makes life a lot easier.

[guest509]

  You guys have totally moved beyond me now. I was able to spot the initial mistake but man I need to go back to school or something.

[Shaggy]

Option Explicit requires explicit declaration of variables...

Option Strict prevents narrowing conversions (if you assign an Integer value to a variable of type Byte, it'll give a compile time error)...

Option Infer allows the compiler to guess a variable's type from a value declaration...

Having Explicit On, Strict On, and Infer Off prevents potential declaration, conversion and type casting bugs before they can take place at run time.

Ahh that makes sense. Thanks for the tip!

Another thing I've seen around, but lack the knowledge of how to use.

List(of T) is a collection of any object type. If you define a loot list and an inventory list, you could more or less write the GetItem routine like this...

I tried doing things like Is Nothing, or = NOITEM [My empty item], but it pulls up an error, and the way I'm doing now works fine for the time being at least

'Nothing' is assigned differently than it's tested...

Code: [Select]

If Not LootMap(Player.Left, Player.Top, intloop).Name = "Nothing" Thento

Code: [Select]

If Not LootMap(Player.Left, Player.Top, intloop) = NOITEM Then

Yeah, that's what I tried doing. So from what I understand, lists are basically arrays, but somehow better? Because right now lootmap is a 3 dimensional array of the class item, which to me seems like it should be the same as a 3 dimensional  list(of item), but apparently is not.

I think you should write a separate sub, but that's because I'd define some kind of MessageDisplay...

Gotcha.

VB.NET is quite different from VB6. It's more solidly object oriented, and of course it uses the .NET library. If you're not familiar with .NET, I'd suggest you read up on it. It's huge, both in its capabilities and its complexity, so it takes some time to find your way around, but after that it makes life a lot easier.

I could definitely use some touching up, thats for sure :p

[Rabiat]

So from what I understand, lists are basically arrays, but somehow better? Because right now lootmap is a 3 dimensional array of the class item, which to me seems like it should be the same as a 3 dimensional list(of item), but apparently is not.

Lists are basically arrays, and they're not necessarily better, but they're dynamic. Arrays are very useful for handling fixed numbers of objects, but cumbersome for varying numbers of objects. It makes sense to implement a level map as a 2D array of tiles, because map dimensions are (usually) fixed. It may also make sense to implement the player's inventory as an array of items, for example if each inventory position is an item slot with a letter assigned to it, but you'll have to take into account that inventory positions may be empty (i.e. contain a NOITEM).

Implementing a loot map as a 3D array, even though it's logical, hardly makes sense.

The map's dimensions (80x25 for example) determine two dimensions of the loot map. The third dimension represents the maximum number of items per tile (let's say 3). If you define the loot map as a 3D array, it allocates 6000 item slots in memory. How many of these are going to be occupied by items? If you scatter 60 objects across the map, they only occupy 1% of the space that's allocated for them. On the other hand, if you want to drop more than three items in one map location, you can't. So a 3D array is a big waste of space in the first two dimensions, and too restricted in the third.

If the loot list is defined as a List(Of Item), the list grows as needed, so it doesn't waste space, and there's no need for NOITEM objects. The only problem is that you can't use map coordinates to find the loot at a given location. It's possible to create a 3D list - a List(Of List(Of List(Of Item))) - but many of the lists in the first two dimensions would be empty. A solution for the first two dimensions would be to use a Dictionary(Of TKey, TValue). A Dictionary is essentially a hash list, which uses keys in stead of indices. This way you can use the X and Y coordinates of map locations as hash keys for the loot list.

To make it more practical, encapsulate it in a LootList class:

Code: [Select]

Class LootList
    Inherits Object

    Private items As Dictionary(Of Integer, Dictionary(Of Integer, List(Of Item)))

    Public Sub New
        items = New Dictionary(Of Integer, Dictionary(Of Integer, List(Of Item)))
    End Sub

    ' Returns a list of items at map location (x, y), or Nothing if no items are there.
    Public Function GetItems(ByVal x As Integer, ByVal y As Integer) As List(Of Item)
        If items(x) Is Nothing Then
            Return Nothing
        Else
            Return items(x)(y)
        End If
    End

    ' Returns a single item at map location (x, y), or Nothing if no item is there.
    Public Function GetItem(ByVal x As Integer, ByVal y As Integer, ByVal z as Integer) As Item
        If items(x) Is Nothing Then
            Return Nothing
        Else If items(x)(y) Is Nothing Then
            Return Nothing
        Else If z < 0 OrElse z >= items(x)(y).Count Then
            Return Nothing
        Else
            Return items(x)(y)(z)
        End If
    End

    ' Puts an item on the loot map at location (x, y).
    Public Sub PutItem(ByVal x As Integer, ByVal y As Integer, ByVal item As Item)
        If items(x) Is Nothing Then items(x) = New Dictionary(Of Integer, List(Of Item))
        If items(x)(y) Is Nothing Then items(x)(y) = New List(Of Item)
        items(x)(y).Add(item)
    End Sub

    ' Removes an item from the map at location (x, y).
    Public Sub RemoveItem(ByVal x As Integer, ByVal y As Integer, ByVal z As Integer)
        If items(x) IsNot Nothing _
        AndAlso items(x)(y) IsNot Nothing _
        AndAlso z >= 0 AndAlso z < items(x)(y).Count Then
            Return items(x)(y).RemoveAt(z)
            If items(x)(y).Count = 0 Then items(x).Remove(y)
            If items(x).Count = 0 Then items.Remove(x)
        End If
    End Sub

End Class

No more NOITEMs, and a lot less looping.

[Z]

Of course storing the contents of a cell as a List is much better than an array of size 3, but isn't using a Dictionary of Dictionaries of these list actually a bad thing in this case? I don't know the details for .NET, but hashtables take some space (and some time to calculate hash values and initialize tables), especially when we have two levels of them. We probably already store the map as a 2D array for some other purpose, so adding a 2D array of item lists (or better, creating a "cell" object which stores all information about a cell and then have a 2D array of them) makes sense to me.

[Rabiat]

You could be right about the 2D dictionary. I've seen several sparsely populated dictionaries up to six dimensions with millions of objects work surprisingly quickly, so I'm (subjectively) convinced the time cost won't be a problem. But I'm not as sure about the memory overhead.

If someone's really interested, we could run alternative implementations through a profiler to compare memory usage and performance. If we're going into that much detail, I'd like to try a Dictionary(Of MapCoordinate, List(Of Item)), where MapCoordinate is a Structure containing X and Y. But I'm not suggesting we should.

My original point, however, was that we shouldn't allocate space for items if we're not going to use it. The same goes for allocating lists of items we're not using. There are usually many cells that simply can't hold items (like walls, or water), and it's wasteful to define an item container for such cells. That's why, in my opinion, a loot list (or a NPC list, for that matter) should be separated from the map itself.

[Z]

Yes, Dictionary(Of MapCoordinate, List(Of Item)) makes sense, I think. Actually, as they say, premature optimization is the root of all evil. Optimizing usage of time and space is useful only when it turns out that the game runs too slowly (which is very unlikely, unless one has huge maps, some specialized AI, or really does not care), or for educational purposes. For now, use something which makes sense and is easy to program. My preferred design (based on Hydra Slayer, although it has one item per cell, so it is simpler) is

• map: a 2D array of map cells
• monster list: a list/vector of monsters (I don't know how is vector in VB), monster knows its location
• map cell: an object which contains a type of terrain feature on this cell, a pointer to the monster, a list of items on this cell, and whatever else information you need (exploration state etc.)

You get a redundancy by having the same information twice (each map cell remembers it monster, which also could be obtained from the monster list), but this redundancy does not seem to be a big problem. All frequently checked for data is available right away, so it is very fast to program and very fast to run. And memory is also very good (assuming a small map of 80x25 and 64 bits per pointer, two extra pointers per each map cell is 32 kB, which is almost nothing).

[Shaggy]

Yes, Dictionary(Of MapCoordinate, List(Of Item)) makes sense, I think. Actually, as they say, premature optimization is the root of all evil. Optimizing usage of time and space is useful only when it turns out that the game runs too slowly (which is very unlikely, unless one has huge maps, some specialized AI, or really does not care), or for educational purposes. For now, use something which makes sense and is easy to program.

That's what I was thinking, because in the future if I get other errors with the lists I'll have more trouble debugging them on mine own due to not being familiar with them.

My preferred design (based on Hydra Slayer, although it has one item per cell, so it is simpler) is

• map: a 2D array of map cells
• monster list: a list/vector of monsters (I don't know how is vector in VB), monster knows its location
• map cell: an object which contains a type of terrain feature on this cell, a pointer to the monster, a list of items on this cell, and whatever else information you need (exploration state etc.)

You get a redundancy by having the same information twice (each map cell remembers it monster, which also could be obtained from the monster list), but this redundancy does not seem to be a big problem. All frequently checked for data is available right away, so it is very fast to program and very fast to run. And memory is also very good (assuming a small map of 80x25 and 64 bits per pointer, two extra pointers per each map cell is 32 kB, which is almost nothing).

that's almost exactly what I was doing originally. I might go back to doing a single item on every cell for now, because switching to multiple per cell seems to be where my problems came up.

[Rabiat]

Actually, as they say, premature optimization is the root of all evil. Optimizing usage of time and space is useful only when it turns out that the game runs too slowly (which is very unlikely, unless one has huge maps, some specialized AI, or really does not care), or for educational purposes.

While I agree with you on the subject of premature optimization, I don't see how it applies to the point I made earlier, or my examples. I wasn't proposing an optimization, I was addressing what I perceive to be a flawed design decision. (I'm not just addressing Shaggy when saying that, it's actually very common.)

For now, use something which makes sense and is easy to program.

If we were dealing with a language in which we'd have to write our own linked lists, hash tables, etc., I'd agree with you. But in this case, we're dealing with .NET, which provides all these things, and makes them as easy to use as more fundamental programming concepts.

Redundancy, as you're suggesting, has its own challenges, especially when debugging two versions of the same data that are somehow inconsistent.

That's what I was thinking, because in the future if I get other errors with the lists I'll have more trouble debugging them on mine own due to not being familiar with them.

I don't mean to be disrespectful, and please don't think I'm trying to convince you to use something you're not familiar with, but your familiarity with Lists and Dictionaries is not necessarily a measure for their actual complexity. My personal experience is that they make coding and debugging easier, not harder.

This does imply, however, that I've derailed your thread towards a subject you feel uncomfortable with. I didn't intend to do that, so I'll stop bothering you with the framework evangelism for now.

[Z]

While I agree with you on the subject of premature optimization, I don't see how it applies to the point I made earlier, or my examples. I wasn't proposing an optimization, I was addressing what I perceive to be a flawed design decision. (I'm not just addressing Shaggy when saying that, it's actually very common.)

I agree that we had a flawed design decision, I just think that it was only partially flawed (storing a list of items in a particular cell as an array of size 3 instead of a List).

If we were dealing with a language in which we'd have to write our own linked lists, hash tables, etc., I'd agree with you. But in this case, we're dealing with .NET, which provides all these things, and makes them as easy to use as more fundamental programming concepts.

If something is hard to do with arrays and easy with linked lists or hashtables, then yes, go and use lists and hashtables. However, the structure for holding all items on a map could be done both with arrays and with dictionaries, and IMO arrays are faster and it is easier to work with (even with redundancy, which is easily solved by never writing redundant data directly and just using a "move monster" method which correctly updates both copies), so in this particular case I suggest to use them. I hope that, after our discussion, Shaggy will know what Lists and Dictionaries are for, and if a situation arises when a Dictionary is essential for elegant, fast, and bug-prone design, he will learn how to use it, and do use it, instead of an array with an arbitrarily chosen limit.

that's almost exactly what I was doing originally. I might go back to doing a single item on every cell for now, because switching to multiple per cell seems to be where my problems came up.

Do as you wish, multiple items per cell is not essential for a good game, DoomRL (among others) has one item per cell and has lots of fans. It has advantages of simplified UI and programming. Disadvantages are, less realism, and that you need to do weird things when you want to drop an item and there is not enough space.