4
\$\begingroup\$

I've recently started to learn Python and the task was to create a "hot or cold" game. They gave me the basic structure for the code and so far so good. But the bonus objective is to find a way to display how long it takes the player to find the block or even limit the amount of time they have to play the game.

I came up with basic ideas of what I want the player to see:

  • 30 seconds to find the block
  • 15 seconds left
  • 5 second left countdown
  • Too late

After several trials it's now working but somehow this looks a bit chaotic to me. Is there another simple way to make this code more readable/efficient? Thanks a lot!

P.S: If found in time, the result is (timer-1) because the function timer += 1 starts at the very beginning of the loop and therefore adds an extra second to the timer. I'm aware this can be written in a better way...

from mcpi.minecraft import Minecraft
mc = Minecraft.create()

import math
import time
import random

destX = random.randint(600, 700)
destZ = random.randint(210,310)
destY = mc.getHeight(destX, destZ)

block = 57

mc.setBlock(destX, destY, destZ, block)
mc.postToChat("Block set!")

timer = 0
mc.postToChat("You have 30 seconds to find the diamond block!")

while True:

   time.sleep(1)
   timer += 1
 
   pos= x,y,z = mc.player.getPos()
   distance = math.sqrt((x - destX) **2 + (z - destZ) **2)

   if timer == 15:
      mc.postToChat("15 seconds left")
   elif 25 <= timer < 30:
      mc.postToChat(30-timer)

   elif timer == 30:
      mc.postToChat("Too late!")
      break

   elif timer != 30:
      if distance > 100:
         mc.postToChat("Freezing")
      elif distance > 50:
         mc.postToChat("Cold")
      elif distance > 25:
         mc.postToChat("Warm")
      elif distance > 12:
         mc.postToChat("Boiling")
      elif 3 < distance < 12:
         mc.postToChat("On fire!")
      elif int(distance) in range(0,3):
         mc.postToChat("Found it in " + str(timer-1) + " seconds!")
         break
\$\endgroup\$
1
  • \$\begingroup\$ The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$
    – BCdotWEB
    Commented Jun 20, 2023 at 8:44

1 Answer 1

1
\$\begingroup\$

Nice little game. Simple, fun. I like it.

simpler and more efficient?

I'm not sure there's a lot of improvement to be had, here. Given that our frame rate is 1 FPS, it's not like we need to re-write in C++ to make it hum.

The big high-level critique would be: Extract Helper! That is, find chunks of code that go together, and break them out as separate functions. That's fine, you'll get to def soon, I have confidence.

You could also use lint tools like ruff or black, but that's a minor concern ATM.

Overall, you have chosen identifiers that are clear and self-explanatory.


random location

You have some crystal-clear code for picking the "needle" destination in the haystack.

Pep-8 asks for names like dest_x rather than destX -- no biggie.

It would make sense to extract these three lines into a small helper function.

Sometimes the needle is in an "easy" location, it's in plain view from far away. If you wanted to make the algorithm a little fancier, you might do something called rejection sampling. Suppose we want to throw random darts at circular target. Easy: uniformly pick random x on the interval -1 .. 1, similarly random y. Compute radius r to central bullseye with pythagoras. And if r > 1, we reject the point as not being within the unit circle, we go on to keep sampling new points. In your case, we might pick a random (x, z) point and write a function which examines the neighborhood and asks: is the point "easy"? Is it in the middle of a flat plain? Or is it down a well or atop a mountain, hard to get near? So you might keep rolling random numbers until an "interesting" needle point is generated.


Putting the rest of the init code inside a function would also be useful. Sooner or later you're going to want to write unit tests which will need to import this file, and we want no side effects during that process, such as the "30 seconds!" announcement. Not something you need to worry about just yet.


accumulated timing errors

   time.sleep(1)
   timer += 1

Ok, here's a bit of a nerdy digression, which you might just ignore for now.

tl;dr: No changes recommended ATM.

Please understand that if we look at the guarantees it makes, the sleep is actually sleeping for 1 + epsilon seconds. There might be other tasks scheduled on your computer, and the sleep() is eligible to wake up after 1.0 seconds, but it might take a little longer for the OS to get around to finishing up with another task and then scheduling your task. Plus it takes you more than zero seconds to compute a message and send it.

So there is an error term here, which accumulates on each iteration. Imagine this game lasted for some minutes, and it was important to get the player to the train station on time before a scheduled departure. We'd be sad for errors to accumulate and arrive just as the minecraft train pulled out of the station.

A standard approach to dealing with such scheduling uncertainty is to see how long until next deadline, typically less than 1 second, and request that much delay:

t0 = time.time()
while True:

   timer += 1
   time.sleep(t0 + timer - time.time())

The current exercise certainly doesn't require that level of fidelity.


Kudos for the x, y, z tuple unpack when getting position of player, very nice. If you had run any linters over this code you probably would have noticed that pos is unused and may be safely deleted.


consistent comparisons

We have two things going on within this loop, and I recommend you break out two helpers:

  • def report_on_time()
  • def report_on_distance()

I like the 25 <= timer < 30 expression, and I like how the timer tests are organized in sequence.

I'm a little concerned about one aspect of game play. In the final five seconds, it appears we send a countdown and we don't send hot/cold advice, which seems bad for the poor panicked player who is wandering around blindly.

When I look at the distance tests it seems we want a helper like this:

def distance_description(distance):
      if distance > 100:
         return "Freezing"
      elif distance > 50:
         return "Cold"
      elif distance > 25:
         return "Warm"
      elif ...

The the loop could just do mc.postToChat(distance_description(distance)).


covering all cases

Let's focus on the end of that distance description.

      elif distance > 12:
         mc.postToChat("Boiling")
      elif 3 < distance < 12:
         mc.postToChat("On fire!")
      elif int(distance) in range(0,3):
         mc.postToChat("Found it in " + str(timer-1) + " seconds!")

Above I complimented your 25 <= timer < 30 chained comparison as being appropriate and expressive. But here I'm going to criticize the 3 < distance < 12 expression as being out-of-place, inconsistent with the previous logic. It would have been enough to say

      elif distance > 3:

given that other cases were already dealt with by the clauses above.

      elif int(distance) in range(0,3):

Ok, two criticisms of that line. It's not consistent with the previous clauses, for no reason. And worse, it means elif 0 >= distance > 3:. Which might be fine. But hey, what happened to 3?!? When distance is exactly 3 we offer the player no advice at all. (This sometimes is masked by the "final five seconds" effect noted above.)


         mc.postToChat("Found it in " + str(timer-1) + " seconds!")

Relying on timer is not so bad. But if we cared about accumulated sleep timing error, or wanted to report the result in tenths of a second for tie-breaking on some leader-board, then asking for the current time() would make sense. Plus we'd want to sleep much less than one second for each time through the loop.

You might choose to update at a higher rate, say FPS = 10 ten frames per second. Sleep for ~ 100 msec each time through the loop, and produce a combined time and distance message, outputting it with

   if prev_msg != current_msg:
      mc.postToChat(current_msg)
      prev_msg = current_msg

Higher frame rates offer a better opportunity for breaking leader-board tie scores.


This code mostly accomplishes its design goals, and suggests avenues for exploration.

I would be willing to delegate or accept maintenance tasks on this codebase.

\$\endgroup\$
2
  • \$\begingroup\$ Wow, this in incredibly helpful and thorough, thanks a lot! I shall try to keep my code coherent next time and avoid different comparison models. Also I"m gonna look into the time.time() method. @J_H \$\endgroup\$
    – Spirit
    Commented Jun 20, 2023 at 12:31
  • 1
    \$\begingroup\$ @Reinderien, true enough, thanks for calling out my laziness. I started to go down the "more than 1 frame per second" path and then avoided taking a beginner down the rabbit hole. I'll try to improve it a bit. \$\endgroup\$
    – J_H
    Commented Jun 20, 2023 at 18:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.