45 replies to this topic

[AlexDiru]

Hi, I'm Alex Spedding

 

I'd like to help in the development of TDM if I'm needed. I'm near the end of a PhD in Computer Science and have a lot of time on my hands before I start a job. I've always wanted to create a stealth game, and this morning I checked out my game I was making a year ago (Unreal Engine) and it was a complete mess and made me sad and then I closed Unreal Engine.

 

Then I looked into writing my own engine - which would take way too long. Then I realised I can offer my skills to an open source project so why not this one!

 

Currently I've just downloaded the latest stable build and got 4 out of 5 building (not MayaImport), and I'm going to look through the code and try to understand the basic structure of it all.

 

Regarding my previous work my Portfolio, GitHub and Twitter are available. Though for the latter I haven't done any gamedev for a while so it's not as good as the other too.

 

Also if there are any books you'd recommend regarding the idTech Engine or just stuff that will help for learning the codebase, please suggest away

[Springheel]

Welcome!  I'm sure some coders will chip in soon.

[AlexDiru]

Thanks

 

I'm looking to fix this bug for now as practice http://bugs.thedarkm...iew.php?id=4587

 

Do you (or anyone else) know of any missions which have items you can't drop that you start with? The included missions don't seem to have any I can test with

Edited by AlexDiru, 29 July 2017 - 02:44 PM.

[grayman]

Welcome!

 

The best way to get started is exactly what you're doing: pick a bug and see if you can fix it.

 

As for test maps where the problem occurs, you could try sending a PM to kingsal to ask him which map he was playing when he encountered the problem.

[AlexDiru]

Awesome Sent him a PM

[AlexDiru]

Ok so I have a minor fix for this problem, I can't really go any further until I have more confirmation of how things should be organised.

 

In "test.def", the entity which shouldn't be droppable is defined as:

entityDef veil_stone
{
    "inherit"                        "atdm:playertool"
    "editor_displayFolder"            "interactables/special"
    "editor_usage"             "The Stone"
    "editor_color"                    "0.078 0.914 0.600"
    
    // model
        "model"                          "models/volta/special/veil_stone.ASE"
        "density"                    ".5"
        "bouncyness"                "0.01"
        "mass"                        "20"
    
    //Inventory
    "frob_distance"                    "75"
    "inv_droppable"                    "0"
    "notPushable"                    "1"
    "inv_name"                        "The Stone"    
    "inv_icon"                        "guis/assets/hud/inventory_icons/icon_stone"
}

And in the .map file there is:

// entity 16
{
"classname" "veil_stone"
"name" "veil_stone_2"
"inv_map_start" "1"
"origin" "89 -94 86"
"rotation" "1 0 0 0 1 0 0 0 1"
}

Items are loaded into the shop using CShop::AddPersistentStartingEquipment() in particular:

if (!itemMerged)
{
	CShopItemPtr anItem(new CShopItem(*found, quantity, 0, false));
	bool canDrop = itemDict->GetBool("inv_droppable", "1"); //This is always returning true (by default) - the dictionary cannot find "inv_droppable" or "inv_icon" or etc

	anItem->SetCanDrop(canDrop);

	_startingItems.Append(anItem);
}	

However, the itemDict refers to the entity defined in the .map file rather than the .def file. So if "inv_droppable" "0" is added to the .map file entity, the drop button will be removed for it

// entity 16
{
"classname" "veil_stone"
"name" "veil_stone_2"
"inv_map_start" "1"
"origin" "89 -94 86"
"rotation" "1 0 0 0 1 0 0 0 1"
"inv_droppable" "0" <----- I can't make this bold but this is the key line
}

Now I'm not sure whether the inv_droppable should be defined in the .map entities or the .def entities, so I need some confirmation regarding this

 

And now I need to go to bed now, it's past 2AM 

Edited by AlexDiru, 29 July 2017 - 08:02 PM.

[grayman]

I'm a little confused.

 

I assume this is from Volta and the Stone.

 

Since one objective in the mission is to find and steal "The Stone", why is it marked "inv_map_start" "1". That says the player has it in his inventory at mission start, which is obviously not the case.

 

And CShop::AddPersistentStartingEquipment() is called in a campaign for mission N to carry over persistent items from mission N-1.

 

Does anyone know what's supposed to happen here?

[grayman]

Okay, I see that Volta and the Stone is the first mission in a campaign.

 

So is the problematic definition defined in the second map in the campaign, which hasn't been released yet? And the second map is placing the stone in the player's inventory because he found it in the first mission?

[grayman]

I think there's an easier way of looking at this.

 

For inventory items that aren't supposed to be droppable (like the compass and the lockpicks), they get this spawnarg:

 

"inv_droppable" "0"

 

The bug is that these types of items have an active Drop button in the Shop regardless. And if the player clicks on that button, it changes to Take, which means it's no longer in their inventory. But at mission start, it's there anyway.

 

I don't think anyone's ever come across this before because these non-droppable items are non-droppable for a purpose: they're most likely essential to the mission.

 

So w/o getting into the complications of a campaign, any item that's marked "inv_droppable/0" should have neither a Take nor a Drop button in the Shop Gui. The field should be left blank, and made unresponsive to the player keys.

 

Given that, you can test with any map that has a shop. Most missions have compasses and lockpicks in them, so you just have to start a few missions until you find one with a Shop.

[AlexDiru]

But the problem is where should "inv_droppable" be defined? I'm not too use about the structure of things, but if it's defined in the .map file it works, but in a .def file it fails.

 

For example, the lockpicks are defined in tdm_defs01.pk4/tdm_shopitems.def:

entityDef ShopItem_playertools_lockpick_set
{
	"inherit"			"atdm:shopitem_base"
	"editor_usage"		"Lockpicks"
	"displayName"		"#str_02389"	// Lockpicks
	"displayDesc"		"#str_02276"	// A set of tools for opening locks."
    "itemClassname"		"atdm:playertools_lockpick_triangle"
    "itemClassname1"	"atdm:playertools_lockpick_snake"
	"image"				"guis/assets/purchase_menu/lockpicks"
	"price"				"20"
	"stackable"			"0"
}

With no "inv_droppable" attribute.

And in the prologue9.map (A New Job FM), there are no lockpicks defined. So as far as I know lockpicks aren't marked as "inv_droppable", though it's likely just in a different file.

 

With the "inv_droppable" attribute in the .map file for the Volta Test, everything works as expected:

 

[AlexDiru]

I'm gonna play some missions and then look at another bug, because I think I've got as far as I can with this. Needs a better understanding of scripting etc to know where things are supposed to go (in def files or map files). But I've found a workaround and documented everything so hopefully it will help for a full fix if needed

 

Found a fix for this (pretty simple, just removing a few blocks of code): http://bugs.thedarkm...iew.php?id=4499

 

To me, it looks better once changed as it matches the download list, however, I guess this is all personal preference - are bugs reviewed once people submit them?

 

Please could I have permissions added for the repo if you want me to commit this fix/preference?

Edited by AlexDiru, 30 July 2017 - 05:40 AM.

[Destined]

In general, def files globally define entities, while map files define any deviations from these definitions. So, if it is only in a certain map that the item should not be dropped, it is defined in the map file (by the map author), if it should not be dropped in general, it should be defined in the def file.

 

Edit: At least this should be, where goes what. I have to admit that I don't have much experience, when it comes to the shop.

Edit 2: After taking another look at the Shop Wiki Page, it appears that the place of the definition depends on the method, the map author chose to add the item to the shop. If the item is placed in the map and given the spawnarg "inv_map_start" "1", it will apparently appear in the shop. Items added this way can be modified in the map file. Items that are added using the "shopitem_N_item" spawnarg in the shop entity use the definitions of the def files.

Edited by Destined, 30 July 2017 - 05:57 AM.

[AlexDiru]

So based on your Edit 2, the item is "inv_map_start" "1", thus it should be modified in the .map rather than the .def file. Meaning that this isn't actually a bug?

 

Also I'm unable to log into the bug tracker, something about my password being incorrect (which it's not) or my account being disabled :/

Edited by AlexDiru, 30 July 2017 - 06:53 AM.

[grayman]

Wrt "inv_droppable", if it doesn't appear in the spawnargs for the item, or the classes it inherits from, then you check the code to see how it's set up at spawn time.

 

In case of inventory items, we find this line in CInventoryItem::CInventoryItem():

 

m_Droppable = itemEntity->spawnArgs.GetBool("inv_droppable", "0");

 

The "0" in that line says that if the GetBool method can't find a matching spawnarg for "inv_droppable", then it will return a default of "0".

 

So that's how droppability is determined for items that don't explicitly state whether they're droppable or not. They're not.

[AlexDiru]

CInventoryItem's constructor isn't called on the shop set up, but when the actual mission starts, thus it doesn't relate to the shop

[Destined]

No, "inv_map_start" "1" is a spawnarg, i.e. a property that an item has (the values are set, as soon as the item is spawned, hence the name). If you place an item into your map and give it this property, it will add the item to the player's inventory on map start, which is why it also appears in the shop. This option is mostly improtant for items that the map author wanted to change in some way, and thus, deviate from the general definition. If the item is used "as is", it should be added to the shop entity via the spawnarg "shopitem_N_item". This omits the need for creating each item in the map and then adding it to the shop, but does not let you change any properties of these items.

[grayman]

In A New Job, the lockpicks are given to the player by the entity "atdm_shop_1", as part of his starting inventory.

[Destined]

CInventoryItem's constructor isn't called on the shop set up, but when the actual mission starts, thus it doesn't relate to the shop

The Shop Wiki page states, that the code checks the map file for items with the "inv_map_start" spawnarg and adds them to the shop.

[grayman]

 

With the "inv_droppable" attribute in the .map file for the Volta Test, everything works as expected:

 

 

 

That's good. So what exactly did you do to make this happen?

[AlexDiru]

The Shop Wiki page states, that the code checks the map file for items with the "inv_map_start" spawnarg and adds them to the shop.

 

Which means the inv_droppable 0 should be defined in the map file?

[AlexDiru]

 

That's good. So what exactly did you do to make this happen?

 

Initially the map file had the definition for the item you shouldn't be able to drop as:

{
"classname" "veil_stone"
"name" "veil_stone_2"
"inv_map_start" "1"
"origin" "89 -94 86"
"rotation" "1 0 0 0 1 0 0 0 1"
}

I noticed, that the code was looking at this, rather than the definition in the script .def file, so I copied the inv_droppable attribute to here:

{
"classname" "veil_stone"
"name" "veil_stone_2"
"inv_map_start" "1"
"origin" "89 -94 86"
"rotation" "1 0 0 0 1 0 0 0 1"
"inv_droppable" "0"
}

And that made it work as desired on the bug report

 

Edit: double post because I couldn't figure out how to quote two users in the same post

Edited by AlexDiru, 30 July 2017 - 07:11 AM.

[grayman]

Wrt issue 4499, I suggest starting a thread in the General Discussion -> The Dark Mod forum to discuss the requested change.

 

I agree that it's slightly disconcerting that the mission names are listed differently, but that's my opinion. Others might have a different opinion.

 

You need to suss out whether there's a consensus for listing missions as "A blah blah blah" or "Blah blah blah, A", then make the change in the code that doesn't use that form, to switch it to use the form that most people like.

[grayman]

Okay, so wrt "inv_droppable", if it's set to NO ("0") in the map, then the Shop code already takes care of NOT displaying the Drop/Take button?

 

If that's the case, then fixing the Volta map solve's Volta's problem, but it leaves the general case where un-droppable items like the Compass and lockpicks in other maps still get a Drop/Take button that lets the player drop the item but still have it when the mission starts.

[Destined]

 

Which means the inv_droppable 0 should be defined in the map file?

As Grayman stated: the code checks if "inv_dropable" is defined in the map file. If it is not defined, (according to Grayman) it should set it to "0" by default. However, this appears to be not recognised by the code for the shop, as items not added with "inv_map_start" appear to be droppable in the shop (even though they are not in the map). At least, this is how I currently understand it.

 

 

Edit: double post because I couldn't figure out how to quote two users in the same post

 

Whenever you hit the "quote" button, it adds the respective post to position of the cursor in your Reply field. This means, you can simply hit several "Quote" buttons and have all the quotes in your reply field.

[AlexDiru]

Okay, so wrt "inv_droppable", if it's set to NO ("0") in the map, then the Shop code already takes care of NOT displaying the Drop/Take button?

 

If that's the case, then fixing the Volta map solve's Volta's problem, but it leaves the general case where un-droppable items like the Compass and lockpicks in other maps still get a Drop/Take button that lets the player drop the item but still have it when the mission starts.

 

First sentence = Yes

 

Second point, you are correct, I can't see a fix for the general case without a fundamental change of the behaviour of the reading of the files - I think it would be better just to hardcode the fixes, since they are in-built items and people can still control custom items.The hardcode fix will be something like:

CanDrop?() {
   If Name In { "Compass", "Lockpick", "Blackjack", ... }
      Return False
   Behave As Normal From Here On
}