Hi! and welcome! , just noticed that it seems you keep developing on the same linked resource, so when others go to take a look, things might not be in a working state / halfway between 2 features! (this might be discouraging for people to try and debug to get the code into a working state, and then comment on the structure of your code), also when you do get suggestions, and implement the changes, others that come by to read the thread might not follow the thread as easily!
for starters, it seems like you have a fairly good grasp of coding already (you mentioned you already had some experience from 5 years ago, and it shows!), and as such, there aren’t that many obvious mistakes that pop up (in contrast to those that are completely new to coding).
In general, asking for something specific is much more likely to yield answers (I just noticed that nobody answered over the last few days), and it really boils down to the vague “is there something wrong?” type of question!
some ‘naming’ nitpicking:
show() {
if (this.showing) {
here I would maybe choose a better word than ‘show’ for your class method that does the rendering, maybe draw
or render
are good alternatives. Especially when you then use the word ‘showing’ in the next line, as the state for whether the class is to be drawn or not.
if (this.showing) {
I like to mirror the name draw()
library function in my class methods (reducing the vocabulary of my program… even though show()
is a perfectly valid name!).
another name I don’t really like is overBar
:
if (this.overBar) {
it’s inside the Inventory, but it means that “the mouse is over the drag-bar”, so I would name it mouseOverBar
.
another one is locked
:
function mouseDragged() {
if (locked) {
inventory.dragMove();
}
}
here I think it means “the inventory is locked to the mouse”, so I would name it inventoryLockedToMouse
, but I would also consider moving the variable into the Inventory class, since it only seems to relate to the inventory. Once that’s done, the need for “inventory” in the variable name becomes redundant, and the name instead becomes “lockedToMouse” .
I hope this gives a window into how I think when I go about naming things, but it’s not easy…
It’s not without good reason when people say “the two hardest things in programming is ‘naming things’, ‘cache invalidation’, and ‘off by 1’ errors”. Naming comes with practice, and I’m by no means an expert so don’t think the names I chose are “the only right ones”, I’d instead focus on the process of naming. (and it’s so easy to be opinionated about naming)
comments
code ‘decisions’ as comments
//did bug here once? cannot get it go again.
(assuming that this comment alludes to a ‘bug that happened’ at some point in time, but after a change in your code, it stopped happening)
ok, normally I wouldn’t even mention ‘git’ on this forum, as processing/p5js is mostly for ‘quick prototyping’ (and fun), but given that you’re a bit experienced already, and the vagueness of the question, I think this might be worth at least mentioning! but If you end up wanting to grow this application to something bigger, and maybe come back to it in the past, or share it with a friend, then ‘git’ becomes an inevitable subject/hurdle. Some type of comments just ‘disappear’ when you start writing them as ‘changes / decisions over time’, you made the decision at “some point in time”, and it belongs to “some piece of code”, and the place to write a comment linking a decision to the change is… VCS! (version control system) I called it ‘git’ earlier, but that’s just the most common form of VCS out there!
another type of ‘code decision’ as comment is the famous ‘commented out code’:
//item = new Item(itemSheet, 16, x, y, itemID - 1);
//items.push(item);
They have a tendency to ‘stick around’ more than they tend to ever be uncommented again, so once you “make the decision” to “not have this code execute anymore”, just delete it, and let the code exist in your VCS!
comments as code (aka. “repeating what the code does”)
comments such as:
//draw rectangle top bar
should be “written as code”, if a function is becoming too long to ‘read easily’, and feel like they necessitate a comment to describe what it’s doing, it’s very likely that the function itself has become too large, and is doing too many things, here I would split this function up into the following:
drawTopBar() {
...
}
drawItemCells() {
...
}
drawInventory() {
this.drawTopBar();
this.drawItemCells();
}
It might seem like “extra code I have to write, and the comment is just 1 line, whereas a function definition and call requires at least 4 lines!..”, but wait! there are some benefits!
- code never gets “outdated”. Sometimes a comment is left behind when the code changed, and it leads to more confusion than if it hadn’t been there to begin with!
- it’s easier to “see the scope” of related code, the code that “draws the top bar” is separate from code that “draws the inventory”
- code is never wrong, or lies to you, it simply does
- code is interactive! (you can run it and see what happens, and step through it with a debugger)
- refactoring becomes easier, swap out the whole function, or move the function somewhere else! (this point kinda relates to the point about “scope”)
standing on the shoulders of giants (#storytime)
Now yes, I just wanted to use that phrase in the title ;), but we really do “stand on the shoulders of giants” when it comes to code (as with most things), languages come with standard functions, thousands of libraries simply exist, conventions are formed! (and subsequently not followed!), but it’s a wonderful place to be!
I mention this because I struggle with this myself (still), but at some point on my programmer journey I told myself in a sort of strict ‘finger wagging’ way: “spend a lot of time researching your first moves!, it’s not worth it to start writing code too early!”, I fondly remember going off and wanting to “write code!”, and sure I knew how to apply the knowledge I had just learnt from university, and went off creating stuff since “this is what I was trained to do!.. writing code!”
Not once did I stop to think “… what is out there?!.. what do other people do?..” (the worst part about this story is that I sometimes STILL do this!, despite having supposedly “learnt my lesson”).
to make this story relevant here, when you have code like this addItem
(and also removeItem
):
addItem(item, quantity) {
//print("ADDITEM")
this.item = item;
var added = false;
//if item is already in the inventory, add to its quantity.
for (var i = 0; i < this.inventoryCell.length; i++) {
if (this.inventoryCell[i][2]) { //if there is something in cell
if (this.inventoryCell[i][3].ID === this.item.ID) {
this.inventoryCell[i][3].incimentStack(quantity); //this does return a true of false depending on if the stack was met.
added = true;
break;
}
}
}
//if the item was not in the inventory, add it to the next empty slot.
if (!added) {
for (i = 0; i < this.inventoryCell.length; i++) {
if (this.inventoryCell[i][2] === false) { //in the cell is empty
this.inventoryCell[i][2] = true; //cell is now full
this.inventoryCell[i][3] = this.item; //we set index 3 of the cell to the item.
this.inventoryCell[i][3].incimentStack(quantity) //and we then incriment the item by that amount.
added = true;
break;
}
}
}
}
It might be worth noting that Javascript has some good standard Array functions you can take advantage of, such as find(), indexOf(), and findIndex(). The last one (findIndex()
) might come in real handy if you had an array of items, and you wanted to iterate through them to find a specific one! and if you restructured your 2D array to be just a single array of data, you’ll find that you might be able to cut down on a large amount of code this way. (all that 2D array iteration, and 2D array access code)
In this case, you can separate the way you want something to look (a 2D grid of items), from the way you want to represent it in code (a single array of items). This is often hard to do on the fly, and perhaps easier in retrospect. But in general it can be worth thinking about these 2 things separately, and giving them an equal share of thought! (they’re both important, and a change in one can make the entire problem a lot simpler).
Now I’m not saying that doing this refactoring to a 1D array is “the right way” to go for this code (in fact, it might turn out to be complicated for both drawing & mouse interaction, going both from 2d -> 1d (mouse click coordinates), and 1d -> 2d (drawing cell from index to canvas coordinates)), but it’s the thought that counts! (and the incentive is “how can I utilize existing functions like findIndex() here?”)