Thread: Converting MINI-BASIC in MASM over to C++?

"depreciated" is something the government may have done with the local currency. "deprecated" is the word we want...

Never mind, we will remove the old code altogether soon, I'd say.

--
Mats

I'd also like to implement a "stdio-console", which can be used in place of the current console code, and which would allow us to do automated testing of things (obviously "locate" won't work).

I will probably look at that next.

Edit: There are still quite a few functions that are really long, and with long chains of if/else in them - this should really be replaced either with tables or refactored to be smaller functions.

--
Mats

Yes, I agree, waiting for the console IO to stabilize would be a good thing.

[Oh, and don't be too fine about the scalpel-work - we're still in a very unstable state, so Stanley knife or butchers cleaver is fine too!].

Load/save as text is also something we should be able to do - although the built-in editor is "nice", it's also very nice to be able to use a regular text-editor to produce the code.

Not sure how much work I will get done the rest of this week, but we'll see.

--
Mats

>> I've hacked a silly callback mechanism until it can be done right.

I like it. It beats the polling mechanism I had set up.

>> The "PITMAN.MBI" program displays perfectly as do several other programs.

WOOT! That's excellent news.

>> Your `std::string operator & <???> (const std::string &, const <???> &)', while cute, breaks MSVC.

Could another operator be used then (maybe operator::,?). It would be a pain to have to use a named function...

>> My only thoughts here: Sabastini wrote his program under the false pretense of the documentation.

Most likely. I'll see if I can rewrite it to run properly.

One thing I did notice, though, is that the drawing output is slow. Maybe we can we speed things up a bit?

>> I'd also like to implement a "stdio-console"

I'm not exactly sure I understand how that would work, but it sounds like it may be useful. Could you elaborate?

>> Beyond that, we also need "TSAVE" and "TLOAD" to save and load in plain text format.

TSAVE is already supported, and instead of having a TLOAD function I just set it up so that the LOAD function can load text, .mbi, or "raw" minibasic executable code.

>> (Seriously, why can't you save in the middle of writing a program? I don't care if I don't have a matching "NEXT"... if I want to save my work I want to save my bloody work!)

That sounds like a good idea. We can just remove the recompile() call from the load function and instead have it set the 'code_modified' flag to true.

>> support for the "F#" keys;

Why not?

>> input queue (a buffer for the last few lines entered into the editor);

This was actually near the top of my list. Before that happens though, I'd like to modify get() to read a string instead of a char*. I work on that.

>> stepping (hit a key to move to the next instruction);

I thought of that, but just never got around to implementing it. I'll see what I can come up with.

Well, good work! I'll pull the latest changes and have a look.

Well, I don't think either of our show_pad implementations are correct. The comments in the assembly actually say:

It stays effective for the rest of the print, unless changed by another format. If no format specified, 6 positions will be used.

In that case, the easiest thing to do would be to generate a padding reset right after the print statement has been parsed.

I think there may be a problem with the 'strip' functionality as well. It doesn't take into account the screen boundaries, and besides that, I really don't understand why it works the way it does. When I commented it out, both PITMAN and REIGN ran fine, so I just left it that until those issues have been clarified.

One other thing - for some reason, the cursor disappears after a line is executed, but is restored say when an error is reported. Any ideas why it's doing this?

Here's a brief update of the lates changes I've made:
- get() now reads a string object. I didn't add any functionality to return virtual codes, because I wasn't quite sure how to correctly handle that.
- Added input queue feature, which is controlled by up and down arrows. Not thoroughly tested.
- Added a very crude stepping mechanism - could probably be improved.
- Adopted interface style of the original minibasic program.
- window() fixed to return correct status.
- Fixed a memory leak in ScrollUp.
- Other minor tweaks.

<edit>If you get to it before I do, try keeping my "stripping" mechanism and print `strip' spaces. It will not fix one problem, but it may be why "REIGN.MBI" isn't displaying properly while "PTIMAN.MBI" is showing improvement.</edit>

"Well, I don't think either of our show_pad implementations are correct."

You may be right. I'll try to grab an hour here in a few minutes to look around some more, but I probably will not be able to look into anything further until Saturday. (You need to keep in mind that we have to emulate the behavior not what the comments claim the behavior is supposed to be if we want compatibility with existing programs.) I think, but can't be sure, that I'm altering the buffer on the wrong side. One of us will figure it out soon enough.

"In that case, the easiest [...] has been parsed."

I agree. How easy would it be to add an instruction during compilation? I had completely forgotten about six being the default. That may be why some of my tests displayed incorrectly.

"I think there may be a problem with the 'strip' functionality as well."

*sigh*

You've broken my code. There is either some aspect of our systems causing incompatible behavior or you are not noticing the "off-by-one" problems. (Honestly, I hadn't noticed all the "off-by-one" errors myself.) Obviously my implementation is off or I'm fixing the wrong problems, but I'm sure I've made a step the right direction.

I've included a link with an image of my desktop displaying three versions of the program (from top to bottom): the original, my latest push, and your push which commented out the changes to the padding functionality.

I'll list some of the remaining problems (with my changes):

1): the top of the map border is missing (blueish green)
2): the "diamond", "stage", and "move" counters (yellow and pink) are one column off

Curiously, without my changes the map border is displaying correctly, but the counters are way off position.

To help you find the solution, I'm just eliminating trailing spaces from the padding specified. If "Test "--note the space--was printed I set `strip' to one and difference the specified padding. (If the padding was set to 3 in the "PRINT" call I subtract one to account for the trailing space.) I readily agree, it is probably not the correct approach, but I am not familiar with the source, and I'm almost positive that the problem comes from the padding implementation. As I said, it was a poorly punned hack, and it should be rewritten--even if it did solve the problem completely.

"Fixed a memory leak in ScrollUp."

LMAO!

I was so busy trying to get "PITMAN.MBI" to run I didn't even think to free the the memory. ^_^;

This is what comes from not using RAII in the first place.

"Any ideas why it's doing this?"

There are two reasons as far as I can remember--and I have a very good memory: I'm at fault for one and the other is a side effect of the additional "LOCATE" functionality--the size functionality. (I did look for this, but didn't have time to try and fix it.)

Yea... so... search the source for `cursor' calls. In trying to find the "off-by-one" error I stuck a few extra calls to trace where it was being updated. I may be wrong, but if you've just noticed it I probably didn't get every call removed.

The "LOCATE" implementation seems to always restore the cursor if the call doesn't explicitly set the size value. It should instead store the current state and use that stored value in the emit routine. (The problem seems to be in the compiler.) I'm not familiar with the source, but I think it would be easier to have two "LOCATE" instructions to be interpreted by the virtual machined and decided by the compiler based on the number of parameters used in the call.

"Maybe we can we speed things up a bit?"

(I didn't get to this earlier because I was in a hurry.)

Instead of implementing one `put' in terms of the other `put' we need to use `WriteConsoleOutput'--or whatever it is called--to print an entire string in one or two system calls. At one point I tried to get a "backtrace" from GDB--as opposed to adding tracing code--, but the output was meaningless because the `cursor' functions are called so very many times.

We also need more "stateful dismissal". I'm sure several routines can be altered to eliminate a few calls because they will, in effect, do nothing.

Soma

screen.jpg (240 KB)

Well, I solved the case of the disappearing cursor: one of us (and I'm not sure it wasn't me) set the cursor size to 0 in the interpreter's constructor!

I'm still reviewing your comments and changes - I'll post a reply as soon as I'm done. I've also made a few more minor changes (including a fix to an off-by-one bug I introduced in console's DELETE logic), and I'll be pushing an update soon.

>> The new source from `void emit(opcode_t value)' should be treated with kid gloves. I'm not kidding. Do not change it unless you know exactly what you are doing and have experience with at least the last few generations of GCC and MSVC.

I don't understand why that had to be changed; the template parameter was explicit, so isn't that guaranteed to select the proper function?

>> The padding should freaking obviously be based on the contents of the string being padded.

I still don't get it - I thought that the padding setting simply meant to print X number of whitespace before outputting something?

>> Instead of implementing one `put' in terms of the other `put' we need to use `WriteConsoleOutput'--or whatever it is called--to print an entire string in one or two system calls. At one point I tried to get a "backtrace" from GDB--as opposed to adding tracing code--, but the output was meaningless because the `cursor' functions are called so very many times.

The problem with that is that it would have to be incorporated into the cursor repositioning and scrolling logic, etc. And anyway, after the newest changes the speed issue seems to have vanished (not sure why, though).

>> How easy would it be to add an instruction during compilation?

Trivial (it was actually implemented in the source I pushed yesterday).

>> I didn't try to do anything with "LOCATE" but changing old line 481 from `emit(int32(false));' to `emit(int32(true));' should at least keep it from "blinking".

Doh - of course. Not sure what I was thinking there. We can probably just generate a negative value during parsing to indicate "no change" to the global setting for the visibility.

One more thing: int get(bool, bool) has a bug I think, causing it to return to soon. I have an idea why and will probably fix that by the next update.

We are getting very close to a complete version here. I'm very happy about that.

There are a few things that I think we should consider:
1. does compiler _AND_ minibasic classes really need to inherit the token_scanner class, rather than own one?
2. the huge number of calls to "console::instance()" seems wrong to me. Having an instance in the minibasic class [or wherever it makes most sense to have it] seems a better option - via a pointer would make most sense if we are to support multiple variants.

--
Mats

OK, I've fixed the get() bug.

And after running PITMAN, I can see the difference with the strip mechanism in place. But shouldn't it also do something when printing text? I'll go ahead and push the latest update, but I think that may need a little reworking.

>> does compiler _AND_ minibasic classes really need to inherit the token_scanner class, rather than own one?

Well, I think it just makes the code much cleaner using inheritance. I'm willing to take a vote on that, but I really don't think it would benefit to switching to a "HAS A" design.

>> the huge number of calls to "console::instance()" seems wrong to me. Having an instance in the minibasic class [or wherever it makes most sense to have it] seems a better option - via a pointer would make most sense if we are to support multiple variants.

But you can't have multiple consoles, can you? Come to think of it, it might even make sense to have the minibasic class be derived from the console class - after all, in a sense it "IS A" console.

Originally Posted by Sebastiani

>> does compiler _AND_ minibasic classes really need to inherit the token_scanner class, rather than own one?

Well, I think it just makes the code much cleaner using inheritance. I'm willing to take a vote on that, but I really don't think it would benefit to switching to a "HAS A" design.

But it's still WRONG to do it that way, even if it's easier/cleaner. minibasic still isn't "IS-A" token_scanner, per se. And we certainly shouldn't really have two (different ones) of them when "compiling", do you think?

In fact, it would probably make sense to CREATE a token_scanner when it is needed [e.g. when token_scanner::set() is called].

>> the huge number of calls to "console::instance()" seems wrong to me. Having an instance in the minibasic class [or wherever it makes most sense to have it] seems a better option - via a pointer would make most sense if we are to support multiple variants.

But you can't have multiple consoles, can you? Come to think of it, it might even make sense to have the minibasic class be derived from the console class - after all, in a sense it "IS A" console.

Inheriting certainly will fail the "multiple types of console" thingy - since you can only inherit from ONE class, and in my mind I see us having more than one class - possibly with runtime decisions as which to use. [e.g. pass in "-s" for stdio-based I/O or "-c" for MS Windows "Console" type I/O]. This would definitely REQUIRE that there is a pointer stored somewhere.

There's nothing saying we can't do:

Code:

   myConsole = &console::instance();

or some such.

--
Mats