Temple of The Roguelike Forums

Development => Programming => Topic started by: Shaggy on November 01, 2010, 11:27:46 PM

Title: Celebration / Issue with loot
Post by: Shaggy on November 01, 2010, 11:27:46 PM
So first off, gooooo me! Ha. I got my map systems working, and I have a [very primitve] FoV system in as well! Check it out! Also, let me know what you think of the HUD? Couldn't think of anything else you'd need, but I may have missed something.

http://img684.imageshack.us/img684/704/60110216.png


Now, on to my issue. For some reason, even if there's plenty of room in the players inventory, it says the inventory is full when you try to pick things up off the map. Any idea why? Here's the code [vb.net] for my pickup routine.

Code: [Select]
Public Sub GetItem(ByVal left As Integer, ByVal top As Integer)

        Dim intloop, intinnerloop As Integer
        Dim picked As Boolean

        picked = False

        For intloop = 0 To 2
            If Not LootMap(Player.Left, Player.Top, intloop).Name = "Nothing" Then

                For intinnerlooploop = 0 To 25
                    If Player.Inventory(intinnerloop).Name = "Nothing" Then
                        WriteAt(1, 0, ConsoleColor.White, "Picked up a ")
                        WriteAt(13, 0, ConsoleColor.Yellow, LootMap(left, top, intloop).Name)
                        WriteAt(40, 0, ConsoleColor.Black, "")

                        Player.Inventory(intinnerloop) = NOITEM
                        Player.Inventory(intinnerloop) = LootMap(left, top, intloop)
                        LootMap(left, top, intloop) = NOITEM
                        picked = True
                    Else
                        WriteAt(1, 0, ConsoleColor.Red, "No free inventory space!")
                    End If
                Next

            Else



            End If
            If picked = True Then Exit For
        Next


    End Sub

EDIT ::

I've also done debugging to make sure that the system knows the item is there, which it does, and that it's in the right position on the map.  The inventory[0 to 25] array shows a ton of empty items, and for some reason it still says there's no space.

I triple checked the spelling, as well as trying to check with a different value. But I get the error nonetheless.
Title: Re: Celebration / Issue with loot
Post by: guest509 on November 02, 2010, 06:57:38 AM
 For intinnerlooploop = 0 To 25
                    If Player.Inventory(intinnerloop).Name = "Nothing" Then


  Shouldn't those 2 variables match?
Title: Re: Celebration / Issue with loot
Post by: Shaggy on November 02, 2010, 07:36:30 AM
Ahhh! Thank you  ;D
Title: Re: Celebration / Issue with loot
Post by: Z on November 02, 2010, 11:02:51 AM
It also seems that when e.g. you have items at positions [ 0] to [6] and [25], it displays the "No free inventory space!" 7 times, then "Picked up X" one time, then "Picked up Nothing" 17 times (for values from 8 to 24), then "No free inventory space!" one time. I.e., the loop does not stop after finding the first empty position, and displays "No free inventory space!" for each used position.
Title: Re: Celebration / Issue with loot
Post by: Krice on November 02, 2010, 12:34:08 PM
A funny example how long variable name can be confusing and cause a bug! intinnerloop is a stupid variable name anyways. Learn to use names that tell something! In this case I guess there are 25 (26?) inventory slots. So, better variable name could be inventory_slot, if you insist using long variable names for loops.
Title: Re: Celebration / Issue with loot
Post by: RogueMaster on November 02, 2010, 01:50:34 PM
A funny example how long variable name can be confusing and cause a bug! intinnerloop is a stupid variable name anyways. Learn to use names that tell something! In this case I guess there are 25 (26?) inventory slots. So, better variable name could be inventory_slot, if you insist using long variable names for loops.

I can even tell more: among professional programmers (I mean, programmers that work on an enterprise, etc...) there is a standard name for things when programming, and how to write code. For example:

· Constant = All letters Upper case: TRUE, FALSE, YES, NO, etc... There is a constant that does not follows this always: __VERSION__
· Variables = descriptive name. If made of two or more words, the first word is lowercase, the other words uppercase the first letter: numOfItems, playerHealth, inventorySlots, mapLevel, etc...
· Procedures = Like variables, but uppercase the first word: SendMessage(), PrintMap(), MoveMonsters(), ...

But it's really important to use descriptive names, so the maintenance of the code is done easier. Don't use variables very similar, because it can confuse when writting code. But not with similar letter, also with similar meanings, can lead to confussion: numOfItem is almost like numberOfItem, but also numOfObjects, numOfItems, numOfThings, can lead to cofussion.

And avoid variables like 'a', 'm', 'var1', 'var2', etc... unless just used into a loop like "For i = 1 to 10", etc...
But it seems that there is a certain variable that doesn't follows this always: 'aux', that some people may use as a wildcard for some situations that needs an universal variable, and don't want to declare a new one because they just need to store some data somewhere but for just a single operation that doesn't needs a new variable.

Also comments are extremely usefull (and almost a have-to), because the next time you review your code you can remember what that it does exactly, because after tousand lines of code, it's hard to remember how such piece of code works exactly.

This is also very important so you allow other people to read your code without much problem.
Title: Re: Celebration / Issue with loot
Post by: Shaggy on November 02, 2010, 06:34:50 PM
Thanks everyone! I got it all worked out, including that pesky loop issue  ::)

Here's the new code, if anyone's interested
Code: [Select]
Public Sub GetItem(ByVal left As Integer, ByVal top As Integer)

        Dim intloop, intinnerloop, ItemNum As Integer

        'Steps through to find the first item in the stack
        For intloop = 2 To 0 Step -1
            If Not LootMap(Player.Left, Player.Top, intloop).Name = "Nothing" Then
                'sets Itemnum to represent the items spot in the stack
                ItemNum = intloop
                Exit For
            End If
        Next

        'Steps through the players inventory
        For intinnerloop = 0 To 25
            'Finds the first empty inventory
            If Player.Inventory(intinnerloop).Description = "Empty space." Then
                'Display a 'Picked up a [ITEM]' message
                WriteAt(1, 0, ConsoleColor.White, "Picked up a ")
                WriteAt(13, 0, ConsoleColor.Yellow, LootMap(left, top, ItemNum).Name)
                WriteAt(40, 0, ConsoleColor.Black, "")

                'Put the item in the players first empty inventory spot,
                Player.Inventory(intinnerloop) = LootMap(left, top, ItemNum)
                'Take the item off the lootmap
                LootMap(left, top, ItemNum) = NOITEM
                Exit For
            Else
                'if there's no space, display a message
                WriteAt(1, 0, ConsoleColor.Red, "No free inventory space!")
                Console.Title = Player.Inventory(intinnerloop).Name & " " & intinnerloop

            End If
        Next

    End Sub
Title: Re: Celebration / Issue with loot
Post by: Z on November 02, 2010, 09:19:53 PM
The second loop still does not seem to do what was intended...
Title: Re: Celebration / Issue with loot
Post by: Shaggy on November 02, 2010, 09:51:35 PM
Why's that? It's working fine haha
Title: Re: Celebration / Issue with loot
Post by: Krice on November 03, 2010, 06:51:10 AM
· Variables = descriptive name. If made of two or more words, the first word is lowercase, the other words uppercase the first letter: numOfItems, playerHealth, inventorySlots, mapLevel, etc...
· Procedures = Like variables, but uppercase the first word: SendMessage(), PrintMap(), MoveMonsters(), ...

iThinkIt'SHardToReadSomethingLikeThis, this_style_is_better_and_more_readable.
Title: Re: Celebration / Issue with loot
Post by: Z on November 03, 2010, 07:58:09 AM
OK, the "Exit For" should do the trick. However, I think the line "No free inventory space!" is still displayed as many times as you have items in your inventory (before an empty space is found). Probably it does not matter because it is then covered by the correct message and you do not see it, but still... it would be better for your learning of programming to have your program work in a logical way. Something like (I don't know how exactly a while loop is done in Basic)

Code: [Select]
a = 0
while a < 26 and Player.Inventory(a).Description = "Empty space." do
  a=a+1
wend

if a=26 then 
                WriteAt(1, 0, ConsoleColor.Red, "No free inventory space!")
                Console.Title = Player.Inventory(intinnerloop).Name & " " & intinnerloop
else
                'Display a 'Picked up a [ITEM]' message
                WriteAt(1, 0, ConsoleColor.White, "Picked up a ")
                WriteAt(13, 0, ConsoleColor.Yellow, LootMap(left, top, ItemNum).Name)
                WriteAt(40, 0, ConsoleColor.Black, "")

                'Put the item in the players first empty inventory spot,
                Player.Inventory(a) = LootMap(left, top, ItemNum)
                'Take the item off the lootmap
                LootMap(left, top, ItemNum) = NOITEM
endif

Also, I think the loop variable should have an one-letter name, and it is usually better to define a constant instead of using number 26.
Title: Re: Celebration / Issue with loot
Post by: Rabiat on November 03, 2010, 10:53:17 AM
Shaggy:

I'm assuming you're using VB.NET. Some advice:

Other than that I think .NET is a good choice for developing a roguelike. It'll help you create a game, in stead of winding up with the classic Yet Another Incredibly Abstracted Roguelike Library Only I Will Ever Use. ;)
Title: Re: Celebration / Issue with loot
Post by: Shaggy on November 03, 2010, 08:40:16 PM
Shaggy:

I'm assuming you're using VB.NET. Some advice:

  • It's good practice to enable Option Strict, enable Option Explicit, and disable Option Infer. It's a chore to explicitly declare and initialize all variables, but it'll prevent a slew of hard to detect runtime bugs.
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.

  • Like Übermann, I'd encourage you to use descriptive variable names and abide to coding conventions. In .NET, stick to lowerCamelCasing for variables and UpperCamelCasing for classes and methods; it's what everyone does.
I usually do, it's just I use things like intLoop for variables that are used only for loops.

  • .NET has terrific support for variable size collections, such as List(of T) or Dictionary(of TKey, TValue). If you use these, there's no need to check for absence of items from a loot list, or inventory slots with nothing in them.
Another thing I've seen around, but lack the knowledge of how to use.

  • If you have reason to ignore the previous point, just don't pull stunts like If Inventory(i).Description = "Empty space." or LootMap(...).Name = "Nothing". If you change the description/name of the 'empty' item, these tests will fail. It's safer to assign Nothing to an empty slot, and test for Inventory(i) Is Nothing or LootMap(...) Is Nothing. It's best not to have empty slots at all, though.
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

  • It's usually a good idea to separate 'model' code from 'view' code, i.e. separate the code that actually moves an item from the floor to the inventory, from the code that displays a message. In any case, don't display a message saying the player has picked up an item before the item is actually moved. If somehow the item can't be moved after all, the message is confusing.
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

Other than that I think .NET is a good choice for developing a roguelike. It'll help you create a game, in stead of winding up with the classic Yet Another Incredibly Abstracted Roguelike Library Only I Will Ever Use. ;)
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.
Title: Re: Celebration / Issue with loot
Post by: Ex on November 04, 2010, 03:57:29 AM
· Variables = descriptive name. If made of two or more words, the first word is lowercase, the other words uppercase the first letter: numOfItems, playerHealth, inventorySlots, mapLevel, etc...
· Procedures = Like variables, but uppercase the first word: SendMessage(), PrintMap(), MoveMonsters(), ...

iThinkIt'SHardToReadSomethingLikeThis, this_style_is_better_and_more_readable.
IPreferCamelCase.
Title: Re: Celebration / Issue with loot
Post by: Z on November 04, 2010, 09:49:13 AM
  • .NET has terrific support for variable size collections, such as List(of T) or Dictionary(of TKey, TValue). If you use these, there's no need to check for absence of items from a loot list, or inventory slots with nothing in them.
I think that, if the inventory has a fixed size of 26 items, using an array of size 26 with 'nothing' in some of them is OK (if you assign a letter to each item, which is 'a' plus its index, then the letters won't change even if items in between are dropped, which is good IMO). However, I don't understand why a floor slot would have a fixed size of 3, so either allow only 1 item per floor slot, or use a List or something.

But checking for nothingness by comparing item name to "Nothing" is indeed very ugly. In fact, any checking for item/monster type by comparing strings is ugly. It's best to consider strings as data which can be changed easily (e.g., for the purpose of translation) and thus their exact values should not appear everywhere in your code, only in 1 place. Either use a "type" field which is an integer value, or use an Object Oriented approach, where every item type is a different class.

Quote
Quote
  • It's usually a good idea to separate 'model' code from 'view' code, i.e. separate the code that actually moves an item from the floor to the inventory, from the code that displays a message.
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

No, create a ShowMessage procedure, which displays a message (any message, not only "you pick up an item") to the user. This has many advantages compared to the current solution: it makes changing the layout of the game much easier, and you can also easily remember all the messages so that you can see them with a "show previous 50 messages" key, or log them to an external file. However, there is one disadvantage: it is much harder to do syntax highlighting, that is, displaying parts of the message in different colors (you need to do some design to have both syntax highlighting and the advantages above). Since you already have syntax highlighting, I suppose this could be a problem.
Title: Re: Celebration / Issue with loot
Post by: Rabiat on November 04, 2010, 10:08:02 AM
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.

Quote
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


Quote
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

Quote
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.)

Quote
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. ;)
Title: Re: Celebration / Issue with loot
Post by: guest509 on November 04, 2010, 11:31:22 PM
  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.
Title: Re: Celebration / Issue with loot
Post by: Shaggy on November 05, 2010, 05:42:12 PM
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!

Quote
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...


Quote
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
Title: Re: Celebration / Issue with loot
Post by: Rabiat on November 06, 2010, 11:44:44 AM
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. :)
Title: Re: Celebration / Issue with loot
Post by: Z on November 06, 2010, 08:05:09 PM
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.
Title: Re: Celebration / Issue with loot
Post by: Rabiat on November 07, 2010, 09:14:07 AM
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.
Title: Re: Celebration / Issue with loot
Post by: Z on November 07, 2010, 12:22:13 PM
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


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).
Title: Re: Celebration / Issue with loot
Post by: Shaggy on November 08, 2010, 01:22:03 AM
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.
Title: Re: Celebration / Issue with loot
Post by: Rabiat on November 08, 2010, 07:11:19 AM
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.)

Quote
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. ;)
Title: Re: Celebration / Issue with loot
Post by: Z on November 08, 2010, 08:56:53 AM
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).

Quote
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.