Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/chipinvaders.sv
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,12 @@ module chipinvaders (
// Alien formation
localparam int NumberRows = 5;
localparam int NumberColumns = 8;
localparam [15:0] AlienSpriteWidth = 16;
localparam [15:0] AlienSpriteHeight = 16;
localparam [3:0] AlienScaling = 2;
localparam [15:0] ProjectileSpriteWidth = 1;
localparam [15:0] ProjectileSpriteHeight = 4;
localparam [3:0] ProjectileScaling = 4;
localparam logic [15:0] AlienSpriteWidth = 16;
localparam logic [15:0] AlienSpriteHeight = 16;
localparam logic [3:0] AlienScaling = 2;
localparam logic [15:0] ProjectileSpriteWidth = 1;
localparam logic [15:0] ProjectileSpriteHeight = 4;
localparam logic [3:0] ProjectileScaling = 4;
logic [NumberRows-1:0][NumberColumns-1:0] alive_matrix;
logic [NumberRows-1:0][NumberColumns-1:0] hit_matrix;
logic [NumberRows-1:0][NumberColumns-1:0][15:0] alien_position_x_matrix;
Expand Down
2 changes: 1 addition & 1 deletion src/rtl/alien.sv
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ module alien #(

invert_movement = alive && !frozen &&
(movement_counter >= movement_frequency) &&
(next_position_x+(SPRITE_WIDTH*SCALING_FACTOR) >= MAX_POSITION_X ||
(next_position_x+(SPRITE_WIDTH*SCALING_FACTOR) >= MAX_POSITION_X ||
next_position_x < movement_width);

reached_bottom = alive && (position_y + (SPRITE_HEIGHT * SCALING_FACTOR) >= MAX_POSITION_Y);
Expand Down
6 changes: 3 additions & 3 deletions src/rtl/alien_projectile.sv
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module alien_projectile #(
parameter LOWER_BORDER = 480,
parameter SCALING = 4
parameter logic[15:0] LOWER_BORDER = 480,
parameter logic[9:0] SCALING = 4
) (
input logic clock,
input logic reset_n,
Expand All @@ -23,7 +23,7 @@ module alien_projectile #(
output logic projectile_gfx
);

localparam ProjectileSpeed = 6;
localparam logic[9:0] ProjectileSpeed = 6;

logic frame;

Expand Down
16 changes: 9 additions & 7 deletions src/rtl/cannon.sv
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,28 @@ module cannon #(
$readmemb("src/rtl/single_barrel_cannon_fire.hex", sprite_rom_firing);
end

// --- RENDERING LOGIC ---
logic signed [10:0] rel_x, rel_y;
logic in_sprite_bounds;

always_comb begin
rel_x = (10'(pix_x) - x_reg) / scale;
rel_y = (10'(pix_y) - SHIP_Y) / scale;
rel_x = (pix_x - x_reg) / scale;
rel_y = (pix_y - SHIP_Y) / scale;

Comment on lines 61 to 64

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scale is used as a divisor unconditionally. Even though you later mask output with scale > 0, rel_x/rel_y are still computed via / scale, so scale == 0 causes division-by-zero behavior. Handle scale == 0 explicitly (e.g., set cannon_graphics = 0 and skip the division / bounds logic).

Copilot uses AI. Check for mistakes.
in_sprite_bounds = (rel_x >= 0) &&
(rel_x < SpriteWidth) &&
(rel_y >= 0) &&
(rel_y < SpriteHeight);

// rel_y[3:0] instead of [2:0] - need 4 bits for indices 0–15
if (fire) begin
cannon_graphics = in_sprite_bounds
cannon_graphics = (in_sprite_bounds
? ~sprite_rom_firing[rel_y[3:0]][SpriteWidth-1-rel_x[3:0]]
: 1'b0;
: 0)
& scale > 0; // scale = 0 -> invisible
end else begin
cannon_graphics = in_sprite_bounds ? ~sprite_rom[rel_y[3:0]][SpriteWidth-1-rel_x[3:0]] : 1'b0;
cannon_graphics = (in_sprite_bounds
? ~sprite_rom[rel_y[3:0]][SpriteWidth-1-rel_x[3:0]]
: 0)
& scale > 0; // scale = 0 -> invisible
end
end

Expand Down
9 changes: 5 additions & 4 deletions src/rtl/cannon_display.sv
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,17 @@ module cannon_display #(
$readmemb("src/rtl/single_barrel_cannon.hex", sprite_rom);
end

logic signed [10:0] rel_x, rel_y = 0;
logic signed [10:0] rel_x, rel_y;
logic in_sprite_bounds;

always_comb begin
rel_x = (10'(pix_x) - x_reg) / scale;
rel_y = (10'(pix_y) - CANNON_Y) / scale;
rel_x = (pix_x - x_reg) / scale;
rel_y = (pix_y - CANNON_Y) / scale;

Comment on lines 22 to 25

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scale is used as a divisor unconditionally. When scale == 0 (which the comment says should mean invisible), this will result in division-by-zero behavior (X in simulation / tool-dependent synthesis). Guard the division and in_sprite_bounds calculation with if (scale > 0) (or clamp scale to at least 1) so the invisible case is safe.

Copilot uses AI. Check for mistakes.
in_sprite_bounds = (rel_x >= 0) && (rel_x < SpriteW) && (rel_y >= 0) && (rel_y < SpriteH);

cannon_graphics = in_sprite_bounds ? ~sprite_rom[rel_y[3:0]][SpriteW-1-rel_x[3:0]] : 1'b0;
cannon_graphics = (in_sprite_bounds ? ~sprite_rom[rel_y[3:0]][SpriteW-1-rel_x[3:0]] : 1'b0)
& scale > 0; // scale = 0 -> invisible
end

endmodule
12 changes: 6 additions & 6 deletions src/rtl/cannon_laser.sv
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module cannon_laser #(
parameter CANNON_Y = 470,
parameter UPPER_BORDER = 50,
parameter SCALING = 4
parameter logic[15:0] CANNON_Y = 470,
parameter logic[15:0] UPPER_BORDER = 50,
parameter logic[15:0] SCALING = 4
) (
input logic reset_n,
input logic clk,
Expand All @@ -23,9 +23,9 @@ module cannon_laser #(
output logic laser_gfx
);

localparam LaserSpeed = 6;
localparam LaserWidth = 1 * SCALING;
localparam LaserHeight = 4 * SCALING;
localparam logic[15:0] LaserSpeed = 6;
localparam logic[15:0] LaserWidth = 1 * SCALING;
localparam logic[15:0] LaserHeight = 4 * SCALING;

always_ff @(posedge clk or negedge reset_n) begin
if (!reset_n) begin
Expand Down
14 changes: 7 additions & 7 deletions src/rtl/character.sv
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module character #(
parameter logic [15:0] X_POS = 0,
parameter logic [15:0] Y_POS = 0
) (
input logic [15:0] char_code, // ascii when letter
input logic [7:0] char_code, // ascii when letter
input logic [15:0] hpos,
input logic [15:0] vpos,

Expand All @@ -25,9 +25,9 @@ module character #(

logic is_digit, is_letter;

logic [15:0] letter_index;
logic [4:0] letter_index;

logic [15:0] rel_x, rel_y;
logic [2:0] rel_x, rel_y;

always_comb begin
graphics = 0;
Expand All @@ -36,14 +36,14 @@ module character #(
is_digit = (char_code <= 9);
is_letter = (char_code >= "A") && (char_code <= "Z");

rel_x = (hpos - X_POS) / SCALING;
rel_y = (vpos - Y_POS) / SCALING;
rel_x = 3'((hpos - X_POS) / SCALING);
rel_y = 3'((vpos - Y_POS) / SCALING);

if (rel_y < SpriteHeight && rel_x < SpriteWidth) begin
if (is_digit) begin
Comment on lines +39 to 43

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rel_x/rel_y are truncated to 3 bits before the bounds check. This can cause pixels far outside the 5x5 sprite to wrap (mod 8) and incorrectly satisfy rel_x < SpriteWidth / rel_y < SpriteHeight, effectively repeating the sprite pattern across the screen. Keep rel_x/rel_y wide for the arithmetic and bounds check, and only truncate/cast when indexing the ROM.

Copilot uses AI. Check for mistakes.
graphics = digits_rom[char_code][rel_y][SpriteWidth-1-rel_x];
graphics = digits_rom[4'(char_code)][rel_y][SpriteWidth-1-rel_x];
end else if (is_letter) begin
letter_index = char_code[15:0] - "A";
letter_index = 5'(char_code[7:0] - "A");
graphics = letters_rom[letter_index][rel_y][SpriteWidth-1-rel_x];
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/rtl/hud.sv
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ module hud #(

localparam logic [15:0] LiveW = 16;
localparam logic [15:0] LiveGap = 4;
localparam logic [15:0] LiveScale = SCALE * 2;
localparam logic [3:0] LiveScale = SCALE * 2;
localparam logic [15:0] LiveStep = (LiveW + LiveGap) * LiveScale;
Comment on lines +95 to 96

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LiveScale is narrowed to 4 bits, but it is computed as SCALE * 2 where SCALE is also 4 bits. For SCALE >= 8, this truncates and will produce an incorrect LiveStep and HUD layout. Consider keeping LiveScale wide enough for the multiplication (e.g., 5+ bits) and casting only at the port connection, or add a parameter constraint/assertion that limits SCALE to a safe range.

Copilot uses AI. Check for mistakes.

genvar life;
Expand Down
95 changes: 38 additions & 57 deletions src/rtl/hvsync_generator.sv
Original file line number Diff line number Diff line change
@@ -1,70 +1,51 @@

`ifndef HVSYNC_GENERATOR_H
`define HVSYNC_GENERATOR_H

/*
Video sync generator, used to drive a VGA monitor.
Timing from: https://en.wikipedia.org/wiki/Video_Graphics_Array
To use:
- Wire the hsync and vsync signals to top level outputs
- Add a 3-bit (or more) "rgb" output to the top level
*/

module hvsync_generator(clk, reset, hsync, vsync, display_on, hpos, vpos);

input clk;
input reset;
output reg hsync, vsync;
output display_on;
output reg [9:0] hpos;
output reg [9:0] vpos;
module hvsync_generator (
input clk,
input reset,
output logic hsync,
output logic vsync,
output logic display_on,
output logic [9:0] hpos,
output logic [9:0] vpos
);

// declarations for TV-simulator sync parameters
// horizontal constants
parameter H_DISPLAY = 640; // horizontal display width
parameter H_BACK = 48; // horizontal left border (back porch)
parameter H_FRONT = 16; // horizontal right border (front porch)
parameter H_SYNC = 96; // horizontal sync width
// vertical constants
parameter V_DISPLAY = 480; // vertical display height
parameter V_TOP = 33; // vertical top border
parameter V_BOTTOM = 10; // vertical bottom border
parameter V_SYNC = 2; // vertical sync # lines
localparam logic [15:0] HDisplay = 640; // horizontal display width
localparam logic [15:0] HBack = 48; // horizontal left border (back porch)
localparam logic [15:0] HFront = 16; // horizontal right border (front porch)
localparam logic [15:0] HSync = 96; // horizontal sync width
// verticallogic constants

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment: verticallogic constants looks like it should be vertical constants.

Suggested change
// verticallogic constants
// vertical constants

Copilot uses AI. Check for mistakes.
localparam logic [15:0] VDisplay = 480; // vertical display height
localparam logic [15:0] VTop = 33; // vertical top border
localparam logic [15:0] VBottom = 10; // vertical bottom border
localparam logic [15:0] VSync = 2; // vertical sync # lines
// derived constants
parameter H_SYNC_START = H_DISPLAY + H_FRONT;
parameter H_SYNC_END = H_DISPLAY + H_FRONT + H_SYNC - 1;
parameter H_MAX = H_DISPLAY + H_BACK + H_FRONT + H_SYNC - 1;
parameter V_SYNC_START = V_DISPLAY + V_BOTTOM;
parameter V_SYNC_END = V_DISPLAY + V_BOTTOM + V_SYNC - 1;
parameter V_MAX = V_DISPLAY + V_TOP + V_BOTTOM + V_SYNC - 1;
localparam logic [15:0] HSyncStart = HDisplay + HFront;
localparam logic [15:0] HSyncEnd = HDisplay + HFront + HSync - 1;
localparam logic [15:0] HMax = HDisplay + HBack + HFront + HSync - 1;
localparam logic [15:0] VSyncStart = VDisplay + VBottom;
localparam logic [15:0] VSyncEnd = VDisplay + VBottom + VSync - 1;
localparam logic [15:0] VMax = VDisplay + VTop + VBottom + VSync - 1;

wire hmaxxed = (hpos == HMax) || reset; // set when hpos is maximum
wire vmaxxed = (vpos == VMax) || reset; // set when vpos is maximum

wire hmaxxed = (hpos == H_MAX) || reset; // set when hpos is maximum
wire vmaxxed = (vpos == V_MAX) || reset; // set when vpos is maximum

// horizontal position counter
always @(posedge clk)
begin
hsync <= (hpos>=H_SYNC_START && hpos<=H_SYNC_END);
if(hmaxxed)
hpos <= 0;
else
hpos <= hpos + 1;
always @(posedge clk) begin
hsync <= (hpos >= HSyncStart && hpos <= HSyncEnd);
if (hmaxxed) hpos <= 0;
else hpos <= hpos + 1;
end

// vertical position counter
always @(posedge clk)
begin
vsync <= (vpos>=V_SYNC_START && vpos<=V_SYNC_END);
if(hmaxxed)
if (vmaxxed)
vpos <= 0;
else
vpos <= vpos + 1;
always @(posedge clk) begin
vsync <= (vpos >= VSyncStart && vpos <= VSyncEnd);
if (hmaxxed)
if (vmaxxed) vpos <= 0;
else vpos <= vpos + 1;
end

// display_on is set when beam is in "safe" visible frame
assign display_on = (hpos<H_DISPLAY) && (vpos<V_DISPLAY);
assign display_on = (hpos < HDisplay) && (vpos < VDisplay);

endmodule

`endif
Loading